Bug 165221 - IndexedDB 2.0: Send operations off to the server in batches instead of one at a time
Summary: IndexedDB 2.0: Send operations off to the server in batches instead of one at...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Brady Eidson
URL:
Keywords:
Depends on:
Blocks: 160306
  Show dependency treegraph
 
Reported: 2016-11-30 14:15 PST by Brady Eidson
Modified: 2016-11-30 14:59 PST (History)
4 users (show)

See Also:


Attachments
Patch v1 (2.60 KB, patch)
2016-11-30 14:17 PST, Brady Eidson
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Brady Eidson 2016-11-30 14:15:43 PST
IndexedDB 2.0: Send operations off to the server in batches instead of one at a time

Turns out timer scheduling and/or dispatching to the runloop is expensive.

On a profile of "100,000 writes to an IDB", timer scheduling/firing presented as 10+% of the time.

This patch negates much of that.
Comment 1 Brady Eidson 2016-11-30 14:17:33 PST
Created attachment 295767 [details]
Patch v1
Comment 2 Brady Eidson 2016-11-30 14:43:49 PST
GTK build breakage is not because of this patch.
Comment 3 Myles C. Maxfield 2016-11-30 14:55:09 PST
Comment on attachment 295767 [details]
Patch v1

View in context: https://bugs.webkit.org/attachment.cgi?id=295767&action=review

> Source/WebCore/Modules/indexeddb/IDBTransaction.cpp:412
> +    static const size_t operationBatchLimit = 128;

IIRC Ben had an argument against const variables being static. I can't remember it but I think it was about how static variables have to live in their own memory region and therefore have to be a load rather than an immediate.

> Source/WebCore/Modules/indexeddb/IDBTransaction.cpp:-417
> -            schedulePendingOperationTimer();

I assume you are scheduling the operation for the next chunk somewhere else?
Comment 4 Brady Eidson 2016-11-30 14:56:52 PST
(In reply to comment #3)
> Comment on attachment 295767 [details]
> Patch v1
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=295767&action=review
> 
> > Source/WebCore/Modules/indexeddb/IDBTransaction.cpp:412
> > +    static const size_t operationBatchLimit = 128;
> 
> IIRC Ben had an argument against const variables being static. I can't
> remember it but I think it was about how static variables have to live in
> their own memory region and therefore have to be a load rather than an
> immediate.

"An expert" needs to figure this out for sure, because as is there are 5958 results in 1264 files searching for "static const"

> > Source/WebCore/Modules/indexeddb/IDBTransaction.cpp:-417
> > -            schedulePendingOperationTimer();
> 
> I assume you are scheduling the operation for the next chunk somewhere else?

Correct!

Thanks.
Comment 5 WebKit Commit Bot 2016-11-30 14:59:49 PST
Comment on attachment 295767 [details]
Patch v1

Clearing flags on attachment: 295767

Committed r209157: <http://trac.webkit.org/changeset/209157>
Comment 6 WebKit Commit Bot 2016-11-30 14:59:52 PST
All reviewed patches have been landed.  Closing bug.