Grdb.swift: Cursor, Sequence, IteratorProtocol

[sobri909]

I've got a minor memory leak problem. Which I'm 99.9% sure is just a random dumb line of code somewhere that I haven't spotted yet, entirely unrelated to the db layer. But it's given me a semi plausible excuse to turn my result arrays into lazy collections. Basically I'm bike shedding an unnecessary optimisation, because fun.

I noticed the other day you pushed this commit. Which I took as an indication that you were pre-empting my bike shedding. But it looks like you've stopped just short of the end goal? Unless I'm missing something, I'm not seeing any direct interface for turning a cursor into a Sequence, IteratorProtocol result.

Which isn't a major problem, because you've more than provided all the necessary ingredients to do so with a tiny wrapper. I'm just wondering if there's a reason why you haven't already. Is there some fundamental potential disaster that I'm missing?

As always, thanks for providing me the gateway tools to indulge my entirely unnecessary SQL/ORM feature building desires ;)

[groue]

Hello again @sobri909!

Cursor protocol reference

A cursor is not a sequence or an iterator. Main differences are:

• cursors are reference types
• cursors can't be iterated twice
• cursors' next() method can throw

In a way, GRDB's Cursor protocol fills a hole in the Swift standard library and provide a general interface to external streams. They can model a file, a socket, or... database results.

There is no way to turn a cursor into a sequence or iterator because those can't handle errors.

Yet cursors mimic nearly all behaviors of standard lazy sequences, as you have noticed. foreach, map, dropFirst, you name it, are built-in.

You can turn cursors into Array, and Set:

let c = try String.fetchCursor(db, "SELECT ...")
try Array(c) // [String]
try Set(c) // Set<String>

The introduction of cursors in GRDB was due to the need to provide robust error handling. The iteration of database results can fail in the middle - because of any I/O error, sql function that is given invalid arguments, or whatever. Swift Sequences could simply not support that.

Does this answer your questions?

[sobri909]

There is no way to turn a cursor into a sequence or iterator because those can't handle errors.

What errors are we talking about here? Something that a wrapper couldn't handle internally?

I've just thrown together this wrapper, that seems to be doing the job (I haven't profiled it yet to see if it's achieving any efficiencies though).

public class TimelineItems: Sequence, IteratorProtocol {

    private let store: PersistentTimelineStore
    private let cursor: RowCursor
    private var items: [TimelineItem] = []

    public init(in store: PersistentTimelineStore, cursor: RowCursor) {
        self.store = store
        self.cursor = cursor
    }

    public var isEmpty: Bool {
        return count > 0
    }

    public var count: Int {
        while next() != nil {}
        return items.count
    }

    public var first: TimelineItem? {
        if items.isEmpty { next() }
        return items.first
    }

    @discardableResult public func next() -> TimelineItem? {
        if let row = try! cursor.next() {
            let item = store.item(for: row)
            items.append(item)
            return item
        }
        return nil
    }

}

Can you see any major problems with it, in this basic state? I mean, technically I should be catching the throw in next(), but in most cases a throw there tends to mean there's bigger problems, and the app is better off crashing. Unless there's non fatal throw cases that I'm not aware of?

[sobri909]

Oops. I just realised my isEmpty is totally backwards. Hah. But yeah, that's not really the blocker.

[groue]

Technically, yes, the problem is precisely this try!. Your app can decide to crash, but a general-purpose library like GRDB can't. Maybe your app doesn't use throwing SQL functions. Maybe your app doesn't use database files under iOS data protection which can turn unreadable in the background. Maybe your app doesn't run on macOS, where users can mess with the file system. But some apps do. Only cursors can provide both efficient and safe database access.

[groue]

Reading your code, I suddenly realize that I may have, by mistake, removed the safeguard that checks that cursors are consumed on their dedicated dispatch queue. If you consume them on any other thread (do you?), you'll face issues. When I eventually restore this check, this will trigger a fatal error - because it is a programmer mistake to use cursors outside of a protected database queue (as documented).

[sobri909]

Heh. That class has only had an hour of life so far, so I'm not completely sure how many threads are playing with it. But I'd also consider it a bug for it to be passed between threads. So yeah, I'm with you on that!

So far it seems to be a success in the app. After cleaning up some silly mistakes, it's running smoothly. No crashes, no weirdness. And yeah, it sounds like all the exception cases for that next() fail are cases where I want to crash the app anyway. So I might be in the clear.

I still think you should write your own Sequence, IteratorProtocol wrapper though ;) As long as it's documented with those caveats and warnings, it'd be a handy tool for users who don't fall into any of the edge cases. Although perhaps a support headache for when people don't read the docs, and go ahead with it even when the rules say they can't.

[groue]

(BTW, early GRDB versions had no cursors, and only plain and simple sequences that would crash on the first error. I used to think exactly like you do about reading errors. And time has passed, issues has been opened, problems have been fixed.)

[sobri909]

I'm a big fan of "crash early", rather than handling exceptions. It makes debugging life so much easier.

Although as you say, that's not necessarily a luxury that's available to you when providing a general purpose lib. And possibly not for me either, given that this is going into a slightly less general purpose lib.

But for now I think I can get away with calling it user error if they twist it into an exception situation. Hopefully that will last. It would be a shame to lose the convenience.

[groue]

It's your call! Happy ORM, @sobri909 :-)

[groue]

I still think you should write your own Sequence, IteratorProtocol wrapper though

Absolutely not. As you have shown, it is trivial to write such a wrapper. And shoot yourself in the foot because once you keave the trivial zone, your "safe" iterator and all the brittle cathedral you've built around it will fall in pieces. I don't think cursors are inconvenient. They do exactly the job they are made for. No more, no less. In a plain and boring fashion. The recipe for honest and composable software bricks. Your wrapper has all reasons to exist, but in userland, at the app level.

[sobri909]

A cursor comes with the same risks, if passed between threads, does it not? And the wrapper can catch edge case exceptions internally, report them to console for debugging purposes, and if necessary cause an abort.

I don't think the risks are quite so inflated in the wrapper. The same error cases exist in both, it's just a difference in where and how they're handled. There's no reason why a GRDB wrapper couldn't handle them gracefully and informatively, effectively documenting the cases where it shouldn't be used.

[groue]

A cursor comes with the same risks, if passed between threads, does it not?

No. This is abundantly documented. I can't anything if one decides not to use the information that is provided. I don't have to care either. For cross-thread uses, a use case that GRDB handles because it is so fundamental, use arrays. That, also, is clearly documented. I know that the documentation is long, and boring. But one can not argue as if it did not exist. I'm sorry if so many open source libraries have imprecise and unclear behavior, and some users feel like they can twist them in the way they want. This is not the case here.

[sobri909]

Sorry, I wasn't trying to push you there. I'm just enjoying the lib and playing with what directions it can go in :)

[groue]

I really hope you'll find the foundations you need here! I'll answer any question you have. Without precious talkative users like you and a few others, this project could not exist!

[sobri909]

Aah, now I see what you're saying. The cursor can't leave its read() block, because that block isn't on the current thread. I was mistakenly assuming that read() etc executed on the calling thread, using mutexes to enforce writer / reader concurrency rules. But it appears that's not the case.

GRDB is maintaining its own internal pool of dispatch queues on which it executes transactions, and applies its concurrency rules to those. So once you step outside of the read() block you've stepped off the cursor's queue, and touching it beyond there is inviting disaster. Does that sound right?

Which makes it a bit more difficult to get optimisations out of my wrapper. I think it's going to come down to an awkward trade off between memory consumption and read performance.

Memory consumption can be delayed / deferred / avoided by collecting all the rowIds from the cursor at wrapper init time, and only realising them into objects on demand. But that comes with the performance cost of having to do individual fetches for objects that aren't already present in the local cache.

Although, that said, SQL databases tend to be much better at doing those sorts of inefficient fetches than we tend to assume. But still, it's not going to come for free. There will be efficiency costs.

[groue]

Yes, you understand right. Cursors are close to the SQLite metal, and are intended to be consumed soon after they have been created. That is not documented, and it should be.

Maybe this lib will eventually get some lazy DB accessor, with well-defined concurrent behavior. Today, there is no such API. The only well-defined behavior is for cursors that are consumed in the same reading block where they were created (this, as well, deserves a clearer explanation).

My gut feeling is that an abstraction that hides when values are actually fetched in order to provide whatever optimization needs a deep understanding of app concurrency, the underlying db engine, and how those can be made to match (which is far far far from trivial). This usually end up with managed objects, as in Realm and Core Data. Only managed objects can guarantee with precision the state of the database they come from, and let the host application a tiny chance to control what kind of data it deals with.

This kind of API has been strongly ruled out in GRDB design. See https://github.com/groue/GRDB.swift/blob/master/Documentation/WhyAdoptGRDB.md

There are simply better libs for that (again, Realm and Core Data).

The goal of GRDB is to provide the database accesses most apps need. That's not much: fetchOne, fetchCursor, fetchAll, persistence methods, and a bunch of strong concurrency guarantees that put focus on data consistency, so that an app can perform subsequent fetches without fearing concurrent writes to mess with the results.

The rest is often premature optimisation.

Of course, one has to understand what is data consistency and why concurrent writes are a threat in order to understand how the GRDB concurrency model prevents rare concurrency bugs, race conditions, and other nasty issues that are a pain to fix. But one does not have to understand this in order to use GRDB and profit. One just has to follow lexical rules in order to be protected - that is to say, wrap database accesses inside a queue/pool access/read/write block. Escape this rule, leave the protected blocks, and here be dragons. That's as simple as that.

I don't think this is a problem. On the contrary, I've listed a lot of benefits in the article linked above.

GRDB apps fetch arrays when they need arrays, cursors when they need cursors, etc. It's is recommended that they understand the difference. If they don't, it's recommended that they use arrays. If they eventually meet a performance problem, then they deal with it. It may be fetching from some background queue. It may be learning what a cursor is. It may be learning what a SQLite index is. It may be learning to perform efficient and to-the-point SQL queries. It depends. And it's possible to keep it simple.

I see how the API still opens doors for misuse. This deserves both clearer documention, and more fatal errors ;-)

[groue]

Now there are parts of GRDB that care about delayed loading. More precisely, delayed conversion from raw rows to record types.

That's why the init(row:) initializer of RowConvertible doesn't throw. That's why the Row type hides several flavors of database rows (some have raw and fast raw access to SQLite, while some others contain copies of database content that can be used later). You don't get the same rows when you use Row.fetchCursor(), and Row.fetchAll(), for example. Row arrays are guaranteed to be usable on any thread, and thus contain copies of database values. Row cursors must be consumed in a protected queue, and thus can provide raw and fast access.

The FetchedRecordsController built-in with GRDB uses this in order to lazy load records on demand. It happens today that it holds an array of row copies in memory, but it may well in the future store those rows on a disk cache, and become able to deal with millions of records. That's the kind of optimization I'm talking about. And why records can't throw when they convert to and from rows.

[sobri909]

Yeah, those are the kinds of optimisations I've been playing around with in my head this afternoon :) I've already had a poke around in FetchedRecordsController, and been playing with some ideas.

But as you say, a lot of this is premature optimisation. It was already indulgent bike shedding when I started on it, and now that I've had to scale back my ambitions and keep the cursor inside the read() block, it's looking like a poor trade off between coding time and achieved results. To get the benefits I'll have to do a lot more juggling of intermediates, which comes at a cost.

There's definitely cases with large result sets where the optimisations will be worth it. And I really want to play around with achieving some of them eventually.

It's fairly certain that we'd both come at it from different angles though :) Yours safe and deterministic, mine reckless and unpredictable. But in both cases I think there's some good optimisation potential remaining.

I'll have to put my efforts off for another day though. The added complexity means there's other low hanging fruit I should focus on first. Sigh.

[groue]

Cursor documentation has been enhanced in v2.9.0. Thanks!

[sobri909]

Cursors can not be used on any thread: you must consume a cursor on the dispatch queue it was created in.

I got caught out by this because I assumed that the read {} block executed on the calling thread (which, in retrospect, obviously didn't make sense), rather than being sync dispatched to another thread. So it's probably worth explicitly mentioning that the cursor shouldn't be taken outside of the read {} block (ie the read block is not happening on the caller's thread), to avoid foolish assumptions like my own ;)

[groue]

Excellent suggestion! > https://github.com/groue/GRDB.swift/blob/master/README.md#cursors