89239 2012-06-15 12:12:38 -0700 IndexedDB: Don't do full commit for empty transactions 2012-10-03 16:48:10 -0700 1 1 1 Unclassified WebKit WebCore Misc. 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 jsbell jsbell alecflett dgrogan michaeln tony webkit.review.bot oldest_to_newest 650390 0 jsbell 2012-06-15 12:12:38 -0700 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) 674024 1 153369 jsbell 2012-07-19 16:14:24 -0700 Created attachment 153369 Patch 674026 2 jsbell 2012-07-19 16:15:47 -0700 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. 674941 3 153369 dgrogan 2012-07-20 14:01:04 -0700 Comment on attachment 153369 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? 676154 4 alecflett 2012-07-23 16:34:34 -0700 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. 722971 5 153369 jsbell 2012-09-18 12:17:36 -0700 Comment on attachment 153369 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. 734071 6 jsbell 2012-10-03 15:11:46 -0700 Changed title to "don't do full commit for empty transactions" Read-only transactions can still be non-empty if they include index cleanup. 734087 7 166971 jsbell 2012-10-03 15:26:42 -0700 Created attachment 166971 Patch 734088 8 jsbell 2012-10-03 15:28:48 -0700 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 734092 9 jsbell 2012-10-03 15:29:09 -0700 tony@ - r? 734145 10 166971 webkit.review.bot 2012-10-03 16:13:45 -0700 Comment on attachment 166971 Patch Clearing flags on attachment: 166971 Committed r130338: <http://trac.webkit.org/changeset/130338> 734146 11 webkit.review.bot 2012-10-03 16:13:49 -0700 All reviewed patches have been landed. Closing bug. 734204 12 michaeln 2012-10-03 16:48:10 -0700 Woohoo... nice low hanging watermelon sized 10x improvement in small reads :) 153369 2012-07-19 16:14:24 -0700 2012-10-03 15:26:41 -0700 Patch bug-89239-20120719161424.patch text/plain 1555 jsbell U3VidmVyc2lvbiBSZXZpc2lvbjogMTIzMDA3CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggYmFlNTY0YmNkOTA3YWJj YWM4MDUxZTIzZGYwZjE4M2ZjYzBmYWJmMS4uOTIxZmJjNThmMzQ5NTVlMzE0ZmY1ZjNkNjk4ZjNi OWE5MTlkMGVmNiAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDE4IEBACisyMDEyLTA3LTE5ICBKb3No dWEgQmVsbCAgPGpzYmVsbEBjaHJvbWl1bS5vcmc+CisKKyAgICAgICAgSW5kZXhlZERCOiBEb24n dCBkbyBmdWxsIGNvbW1pdCBmb3IgcmVhZC1vbmx5IHRyYW5zYWN0aW9ucworICAgICAgICBodHRw czovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9ODkyMzkKKworICAgICAgICBSZXZp ZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICBEb24ndCBib3RoZXIgY3JlYXRpbmcg YSBsZXZlbGRiIHdyaXRlIGJhdGNoIGlmIHRoZXJlJ3Mgbm90aGluZyBpbiB0aGUgdHJhbnNhY3Rp b24KKyAgICAgICAgdG8gY29tbWl0LgorCisgICAgICAgIE5vIHRlc3RzIC0gY29tbWl0dGluZyB3 aGVuIHRoZXJlJ3MgZGF0YSB0byBjb21taXQgaXMgY292ZXJlZCBlbHNld2hlcmUuCisKKyAgICAg ICAgKiBwbGF0Zm9ybS9sZXZlbGRiL0xldmVsREJUcmFuc2FjdGlvbi5jcHA6CisgICAgICAgIChX ZWJDb3JlOjpMZXZlbERCVHJhbnNhY3Rpb246OmNvbW1pdCk6CisKIDIwMTItMDctMTggIERpbWl0 cmkgR2xhemtvdiAgPGRnbGF6a292QGNocm9taXVtLm9yZz4KIAogICAgICAgICBGaXggdXAgb2xk IG5hbWUgaW4gUnVsZVNldDo6YWRkUnVsZXNGcm9tU2hlZXQKZGlmZiAtLWdpdCBhL1NvdXJjZS9X ZWJDb3JlL3BsYXRmb3JtL2xldmVsZGIvTGV2ZWxEQlRyYW5zYWN0aW9uLmNwcCBiL1NvdXJjZS9X ZWJDb3JlL3BsYXRmb3JtL2xldmVsZGIvTGV2ZWxEQlRyYW5zYWN0aW9uLmNwcAppbmRleCA4NjJh NWZiZmJiZDVhZWQ5YThlODc1ZmRjYWYzZjMxODEyZGYwYWY2Li44MmNjNGY3MTVlZWQ3N2UyMGZm YTE2NDNkNTM5NDRhY2Q2MjcyMWJmIDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViQ29yZS9wbGF0Zm9y bS9sZXZlbGRiL0xldmVsREJUcmFuc2FjdGlvbi5jcHAKKysrIGIvU291cmNlL1dlYkNvcmUvcGxh dGZvcm0vbGV2ZWxkYi9MZXZlbERCVHJhbnNhY3Rpb24uY3BwCkBAIC0xMjcsNiArMTI3LDExIEBA IGJvb2wgTGV2ZWxEQlRyYW5zYWN0aW9uOjpjb21taXQoKQogICAgIEFTU0VSVCghbV9maW5pc2hl ZCk7CiAgICAgT3duUHRyPExldmVsREJXcml0ZUJhdGNoPiB3cml0ZUJhdGNoID0gTGV2ZWxEQldy aXRlQmF0Y2g6OmNyZWF0ZSgpOwogCisgICAgaWYgKG1fdHJlZS5pc19lbXB0eSgpKSB7CisgICAg ICAgIG1fZmluaXNoZWQgPSB0cnVlOworICAgICAgICByZXR1cm4gdHJ1ZTsKKyAgICB9CisKICAg ICBUcmVlVHlwZTo6SXRlcmF0b3IgaXRlcmF0b3I7CiAgICAgaXRlcmF0b3Iuc3RhcnRfaXRlcl9s ZWFzdChtX3RyZWUpOwogCg== 166971 2012-10-03 15:26:42 -0700 2012-10-03 16:13:45 -0700 Patch bug-89239-20121003152554.patch text/plain 1808 jsbell U3VidmVyc2lvbiBSZXZpc2lvbjogMTMwMjc1CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggNzcxYzBhYmQ4Nzg2Mzcx YjUyYWVhYWIzOWVmODZkYjI2NWRiYjQzMC4uY2FkOTRkNTdlNTFmMmY3Y2U2NWRjNGY0MDJjM2M0 ZGNlNGQ2YTI5MyAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDIyIEBACisyMDEyLTEwLTAzICBKb3No dWEgQmVsbCAgPGpzYmVsbEBjaHJvbWl1bS5vcmc+CisKKyAgICAgICAgSW5kZXhlZERCOiBEb24n dCBkbyBmdWxsIGNvbW1pdCBmb3IgZW1wdHkgdHJhbnNhY3Rpb25zCisgICAgICAgIGh0dHBzOi8v YnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD04OTIzOQorCisgICAgICAgIFJldmlld2Vk IGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgIERvbid0IGJvdGhlciBjcmVhdGluZyBhIGxl dmVsZGIgd3JpdGUgYmF0Y2ggaWYgdGhlcmUncyBub3RoaW5nIGluIHRoZSB0cmFuc2FjdGlvbgor ICAgICAgICB0byBjb21taXQuIE5vdGUgdGhhdCBhIHJlYWQtb25seSB0cmFuc2FjdGlvbiBtYXkg c3RpbGwgaGF2ZSBpbmRleCBjbGVhbnVwIHNvIG1heQorICAgICAgICBub3QgYmUgYW4gZW1wdHkg dHJhbnNhY3Rpb24uCisKKyAgICAgICAgVGhpcyBjdXRzIHRoZSBMb29rdXAyIGJlbmNobWFyayBp biBodHRwOi8vcmV5ZXNyLmdpdGh1Yi5jb20vaHRtbDUtc3RvcmFnZS1iZW5jaG1hcmsvCisgICAg ICAgIGZyb20gNzBzIHRvIDJzLgorCisgICAgICAgIENvdmVyZWQgYnkgZXhpc3RpbmcgdGVzdHMs IGUuZy4gc3RvcmFnZS9pbmRleGVkZGIvdHJhbnNhY3Rpb24tYmFzaWNzLmh0bWwKKworICAgICAg ICAqIHBsYXRmb3JtL2xldmVsZGIvTGV2ZWxEQlRyYW5zYWN0aW9uLmNwcDoKKyAgICAgICAgKFdl YkNvcmU6OkxldmVsREJUcmFuc2FjdGlvbjo6Y29tbWl0KToKKwogMjAxMi0xMC0wMyAgUGF0cmlj ayBHYW5zdGVyZXIgIDxwYXJvZ2FAd2Via2l0Lm9yZz4KIAogICAgICAgICBCdWlsZCBmaXggZm9y IFdpbkNFIGFmdGVyIHIxMzAxNjAuCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9wbGF0Zm9y bS9sZXZlbGRiL0xldmVsREJUcmFuc2FjdGlvbi5jcHAgYi9Tb3VyY2UvV2ViQ29yZS9wbGF0Zm9y bS9sZXZlbGRiL0xldmVsREJUcmFuc2FjdGlvbi5jcHAKaW5kZXggMjY1ODBjMjEwYWZmNzcxNzVi ODY2NjQ5YjY4ZDY5YWE1YTUyNmM2NS4uOGZlM2ZkMDhlZGIyNzliY2UzODQ1MGQ1YzdiOWJiYTAx NzQ3NTk1MyAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvcGxhdGZvcm0vbGV2ZWxkYi9MZXZl bERCVHJhbnNhY3Rpb24uY3BwCisrKyBiL1NvdXJjZS9XZWJDb3JlL3BsYXRmb3JtL2xldmVsZGIv TGV2ZWxEQlRyYW5zYWN0aW9uLmNwcApAQCAtMTI1LDYgKzEyNSwxMiBAQCBib29sIExldmVsREJU cmFuc2FjdGlvbjo6Z2V0KGNvbnN0IExldmVsREJTbGljZSYga2V5LCBWZWN0b3I8Y2hhcj4mIHZh bHVlKQogYm9vbCBMZXZlbERCVHJhbnNhY3Rpb246OmNvbW1pdCgpCiB7CiAgICAgQVNTRVJUKCFt X2ZpbmlzaGVkKTsKKworICAgIGlmIChtX3RyZWUuaXNfZW1wdHkoKSkgeworICAgICAgICBtX2Zp bmlzaGVkID0gdHJ1ZTsKKyAgICAgICAgcmV0dXJuIHRydWU7CisgICAgfQorCiAgICAgT3duUHRy PExldmVsREJXcml0ZUJhdGNoPiB3cml0ZUJhdGNoID0gTGV2ZWxEQldyaXRlQmF0Y2g6OmNyZWF0 ZSgpOwogCiAgICAgVHJlZVR5cGU6Okl0ZXJhdG9yIGl0ZXJhdG9yOwo=