For read-only IDB transactions we go through the full motions of an backing store commit, including creating/writing a (presumably empty) leveldb writebatch. This can be short-circuited, which would eliminate unnecessary work and make the read-only transaction resilient to backing store faults (e.g. disk has gone away but data is still in memory)
Created attachment 153369 [details] Patch
I don't know if we actually want this... the only practical upshot is that if leveldb has entered a bad state due to I/O error the read transaction won't trip it if the data is in an in-memory table.
Comment on attachment 153369 [details] Patch LGTM Bike-shedding: Agreed that the desirability question doesn't have a clear answer. My vote would be to add this code, only because there's a conceivable scenario where a docs user wants to be able to read their docs offline even if they can't write due to some flaw in either our code or docs'. I suppose the downside is that this could make such errors less discoverable. No strong opinion. If you do add this code, could you add a comment indicating how it could matter?
My vote is also to land this - I think this also helps in the case of a database going read-only, filling up, or overrunning quota, right? (Not all of that may kick this in today...) Seems like the downside is a tiny amount of code complexity, but it's pretty darn minor. Also agreed on the comment - the "if (m_tree.is_empty())" does not read "if this is a read-only transaction" to me - though maybe the naming of m_tree is the culprit there. not sure.
Comment on attachment 153369 [details] Patch What happens when we iterate a cursor during a read-only transaction - do we clean up stale entries? Should those be committed or should that wait for a follow-up read/write transaction? If we commit, then this doesn't address the scenario where this is defensive against I/O errors. If we don't commit, all subsequent transactions pay that penalty. Also note that the patch as written checks at a lower level - if index cleanup was done, it would still commit, so it's actually moot for this patch. I'm tempted to land it as-is, but perhaps make the bug title less grandiose.
Changed title to "don't do full commit for empty transactions" Read-only transactions can still be non-empty if they include index cleanup.
Created attachment 166971 [details] Patch
michaeln@ came up with the same patch when looking into performance for read-only transactions: https://groups.google.com/a/chromium.org/d/msg/chromium-html5/FRbwyUgspQ0/2xZkWZv1zQAJ
tony@ - r?
Comment on attachment 166971 [details] Patch Clearing flags on attachment: 166971 Committed r130338: <http://trac.webkit.org/changeset/130338>
All reviewed patches have been landed. Closing bug.
Woohoo... nice low hanging watermelon sized 10x improvement in small reads :)