IDB Cursor continue moves one item at a time
Created attachment 188643 [details] Patch
Comment on attachment 188643 [details] Patch lgtm - thanks! I don't think we have any particular perf tests for this unfortunately.
Comment on attachment 188643 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=188643&action=review > Source/WebCore/ChangeLog:13 > + optimization. Include your performance numbers, even if it's a manual A/B test. I think you had a case where the cost of ~700 continues went from ~500ms to ~80ms ? > Source/WebCore/Modules/indexeddb/IDBBackingStore.cpp:1273 > + bool firstIteration = true; It looks like a follow-up patch could potentially simplify firstSeek()/IteratorState/firstIteration a bit, but I think this is good for now. > Source/WebCore/Modules/indexeddb/IDBBackingStore.cpp:1284 > + if (firstIteration && key && forward) { Maybe add FIXME comment to optimize key seeking for reverse cursors? > Source/WebCore/Modules/indexeddb/IDBBackingStore.cpp:1414 > + return ObjectStoreDataKey::encode( No line wrap here. > Source/WebCore/Modules/indexeddb/IDBBackingStore.cpp:1476 > + return ObjectStoreDataKey::encode( No line wrap here. > Source/WebCore/Modules/indexeddb/IDBBackingStore.cpp:1546 > + return IndexDataKey::encode( No line wrap here. > Source/WebCore/Modules/indexeddb/IDBBackingStore.cpp:1638 > + return IndexDataKey::encode( No line wrap here.
Created attachment 192767 [details] Patch
Tweaked the patch based on feedback. Added perf numbers to CL. Note that I wasn't able to replicate quite as dramatic a speedup as Pankaj communicated in email. With a basic test in a release build in Chrome - over an index w/ 100 items, open a cursor, then seek to item 50 - I saw "only" a 2x speedup. dgrogan@, alecflett@ - one last look?
Comment on attachment 192767 [details] Patch lgtm
Chromium-side performance test posted to: https://codereview.chromium.org/12780007/
tony@ - r? I won't land this until the Chromium perf test is in and has some data collected, though.
Comment on attachment 192767 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=192767&action=review > Source/WebCore/Modules/indexeddb/IDBBackingStore.cpp:1325 > + } else { > + if (forward) > + m_iterator->next(); > + else > + m_iterator->prev(); > + } Nit: You could merge the condition into an "else if/else" with the outer if.
Created attachment 193665 [details] Patch for landing
Comment on attachment 193665 [details] Patch for landing Rejecting attachment 193665 [details] from commit-queue. Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=webkit-commit-queue.appspot.com', '--bot-id=gce-cq-04', 'land-attachment', '--force-clean', '--non-interactive', '--parent-command=commit-queue', 193665, '--port=chromium-xvfb']" exit_code: 2 cwd: /mnt/git/webkit-commit-queue Last 500 characters of output: ebkit-commit-queue/Source/WebKit/chromium/third_party/ots --revision 97 --non-interactive --force --accept theirs-conflict --ignore-externals' in '/mnt/git/webkit-commit-queue/Source/WebKit/chromium' 43>At revision 97. ________ running '/usr/bin/python tools/clang/scripts/update.py --mac-only' in '/mnt/git/webkit-commit-queue/Source/WebKit/chromium' ________ running '/usr/bin/python gyp_webkit' in '/mnt/git/webkit-commit-queue/Source/WebKit/chromium' Updating webkit projects from gyp files... Full output: http://webkit-commit-queue.appspot.com/results/17209397
Created attachment 193687 [details] Patch for landing
Comment on attachment 193687 [details] Patch for landing Clearing flags on attachment: 193687 Committed r146152: <http://trac.webkit.org/changeset/146152>
All reviewed patches have been landed. Closing bug.