New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 675372 link

Starred by 12 users

Issue metadata

Status: Fixed
Owner:
Closed: Jan 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Windows
Pri: 1
Type: Bug-Regression



Sign in to add a comment

IndexedDB operations are throttled to one per second

Project Member Reported by phistuck@gmail.com, Dec 17 2016

Issue description

UserAgent: Mozilla/5.0 (Windows NT 6.1; WOW64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/55.0.2883.87 Safari/537.36

Steps to reproduce the problem:
I have an extension that creates a background tab (of a normal web page), reads (cursor) from its IndexedDB, passes the information to the background page and closes the tab when it is done.
Before Chrome 55, the cursor "success" events were not throttled, which made going over 800+ records very quick (up to a second).
Using Chrome 55, "success" events are fired once per second.

What is the expected behavior?
Unthrottled IndexedDB operations.

What went wrong?
Throttled IndexedDB operations.

Did this work before? Yes 54

Does this work in other browsers? N/A

Chrome version: 55.0.2883.87  Channel: stable
OS Version: 6.1 (Windows 7, Windows Server 2008 R2)
Flash Version: 

I will switch to getAll in the meantime, hopefully it is fast.

I do not attach a test case because it might be intentional (and I do not have a prepared reduced test case, only my extension which needs many factors for it to work).
If this is intentional, then, too bad. If not, I can prepare and attach a test case if you need it.
 
Components: -Blink>Storage Blink>Storage>IndexedDB
Status: Untriaged (was: Unconfirmed)
While it is a regression, I do not think it is such an important regression and therefore the priority is not increased.

Comment 2 by jsb...@chromium.org, Dec 19 2016

Cc: cmumford@chromium.org
That's bizarre - if this was affecting normal pages in the wild we'd have gotten many complaints. Definitely not intentional - please send a test case.


Comment 3 by phistuck@gmail.com, Dec 20 2016

Here is a test case.
Right after loading this unpacked extension, two tabs of example.com will open. One will be the foreground tab, the other will be the background tab.
Each of them adds a few records to a new Indexed Database instance and retrieves the using a cursor.
The foreground one finishes immediately (the tab title changes). The background one finishes after a few seconds (the tab title changes).
Open the console of both of them and you will see the timestamps of the operations printed for you.

Enjoy. ;)
indexed-db-slow-background.zip
1.3 KB Download

Comment 4 by phistuck@gmail.com, Dec 20 2016

I forgot to mention, if it were not clear - switching to the background tab before it finishes will make it finish the operations immediately.

Comment 5 by jsb...@chromium.org, Dec 20 2016

Trippy! I repro w/ phistuck's steps. about://tracing shows the foreground tab completing the whole thing in about 60ms. The background tab's work is stretched over about 7 seconds (!).

The actual IDB tasks - both renderer/browser - take the same amount of time (everything is just a few ms). And phistuck is right - there's about 1 second delay between when the front end receives IDBRequest::onSuccess() and  IDBRequest::dispatchEvent is called.

Comment 6 by jsb...@chromium.org, Dec 20 2016

Cc: dcheng@chromium.org
+dcheng who knows something about frame-related timers

I haven't tracked down the source of the delay yet, though.

Comment 7 by jsb...@chromium.org, Dec 21 2016

Components: Blink>Scheduling
Still haven't found the root cause, but the events are just getting queued in the DOMWindow's EventQueue, which seems to be using a throttled task runner. 

This means walking a cursor (which is a sequence of request -> success event -> request -> success event operations) will be throttled as noted. Issuing requests in parallel will not be.



Comment 8 by jsb...@chromium.org, Dec 21 2016

I suspect this was an oversight because almost nothing but the actual DOM uses the EventQueue. Most asynchronous things (storage APIs, fetch, etc) just dispatch events synchronously when the async message is processed. For historical reasons (related to debugging?) IDB enqueues events instead.

 issue 370160  captures some investigation around changing this

Comment 9 by dcheng@chromium.org, Dec 21 2016

Cc: skyos...@chromium.org alexclarke@chromium.org
+ scheduler people

Does the 1 event / second throttle sound consistent with the timer task queue in SuspendableTimer (which DOMWindowEventQueue uses) being throttled?
Cc: altimin@chromium.org
+altimin@
Status: Available (was: Untriaged)
#9: Yeah, sounds like we are throttling the tasks here because the page is in the background. In general we want to throttle operations like this in background tabs because otherwise its pretty easy for misbehaving pages to run down the battery or spin up the CPU fan.

However, maybe the fact that the tab was created by an extension means we should use a different policy here? Then again, badly written extensions shouldn't be allowed to harm the user experience either IMO.

What do others think?
Forgot to add that besides getAll (which is a good idea), another workaround is to make the tab visible either by bringing it to the foreground or opening it in a new window.
This issue also affects normal tabs (e.g. websites, not only extensions) that are not focused/active.

Found that by incident after trying to implement a cross tab master/slave based shared IndexedDB for a project.

I really think, that a lot of web based apps are having to use cross-tab communication to notify on IDB changes..and this, would cause a lot of performance/UX issues for them.

Test case and the issue I'd created (wronly) for the library I'd used: https://github.com/dfahlander/Dexie.js/issues/442
Yeah, isn't specific to extensions.

Consider something like a mail app in a tab that's trying to sync data with a server, or even just populate the main list when the user tabs away. If backgrounded, suddenly the database operations would slow down by many orders of magnitude.
Another real world use case is... running unit tests.

I'd found out that we had to disabled unit tests (i know unit tests typically should not do "real" stuff, as indexedDB, but ... you can call those integration tests), started failing, because of timeouts (e.g. 2s is the regular/default timeout for Karma and most of the other unit testing libraries).

I realize it won't address all use cases here, but you can start Chrome with --disable-background-timer-throttling to turn off this behavior.

Comment 17 by ralp...@google.com, Jan 20 2017

This is also affecting us adversely.

Note that this can also affect visible tabs. If the hidden tab starts a transaction, and gets throttled, the transaction will not close promptly, and any visible tab that tries to start a new transaction will not be able to take a lock on the previous transaction's object stores.

I think this issue's priority should be bumped up, and the throttling rolled back at least in its current state.
Labels: -Pri-2 Pri-1
Owner: skyos...@chromium.org
Status: Assigned (was: Available)
We are facing a similar issue that IndexedDB action in background tab is not responding. However the issue is only able to be reproducible with Chrome 55, and not reproducible in Chrome 57 (Canary). Has scheduling logic been updated between 55 and 57?
In Chrome 57 throttling starts 10 second after background the page (in Chrome 55 throttling started immediately after backgrounding).
@altimin, thanks for the explanation. Has the throttling update since Chrome 57? We still observe the issue in Chrome 56.
yungcheng.chen@ it appears the throttling change happened in https://codereview.chromium.org/2620743002/ which is part of Chrome 57.0.2979.0. See  bug 649942  for some more details.


Status: Started (was: Assigned)
Sorry for the breakage everyone. I'll land a patch that exempts IndexedDB operations from throttling a first step.
@skyos  Thank you for the quick response.  I am not familiar with the patch process so I have a question.  I assume your patch is going into the Chrome 55 code line.  How long does it usually take for a patch like this to be available in the normal update process?

Comment 25 by phistuck@gmail.com, Jan 24 2017

No, it will go into Chrome 56 at most (but since the fix is not yet available, it might not even go into Chrome 56). Unless a critical need arises (a very high impact or an exploited critical security issue), there will not be another Chrome 55 patch (Chrome 56 is due in several days).
A release is gradually distributed to users, it takes up to about two weeks (maybe more in severe cases) until everyone is pretty much up to date, if I remember correctly.
Going from a fix to a stable release patch can take anywhere from days, to days or weeks, depending on the severity and impact of the fix (or on severity of other fixes that happen to also be included).
Project Member

Comment 26 by bugdroid1@chromium.org, Jan 25 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/05a13309259dd16f38eef41adc7cce55e3d15a05

commit 05a13309259dd16f38eef41adc7cce55e3d15a05
Author: skyostil <skyostil@chromium.org>
Date: Wed Jan 25 12:08:58 2017

scheduler: Make DOM window event tasks unthrottled

This patch avoids throttling events that are routed via
DOMWindowEventQueue. This affects a handful of window-related events,
the most important ones being IndexedDB events. It turns out many web
pages (e.g., Google Docs) rely on IndexedDB transactions to be
relatively fast and start encountering errors if they are throttled too
aggressively while a tab is in the background.

BUG= 675372 

Review-Url: https://codereview.chromium.org/2651933002
Cr-Commit-Position: refs/heads/master@{#446000}

[modify] https://crrev.com/05a13309259dd16f38eef41adc7cce55e3d15a05/third_party/WebKit/Source/core/events/DOMWindowEventQueue.cpp
[modify] https://crrev.com/05a13309259dd16f38eef41adc7cce55e3d15a05/third_party/WebKit/Source/core/frame/DOMTimer.cpp
[modify] https://crrev.com/05a13309259dd16f38eef41adc7cce55e3d15a05/third_party/WebKit/Source/core/frame/LocalDOMWindow.cpp
[modify] https://crrev.com/05a13309259dd16f38eef41adc7cce55e3d15a05/third_party/WebKit/Source/core/frame/SuspendableTimer.cpp
[modify] https://crrev.com/05a13309259dd16f38eef41adc7cce55e3d15a05/third_party/WebKit/Source/core/frame/SuspendableTimer.h
[modify] https://crrev.com/05a13309259dd16f38eef41adc7cce55e3d15a05/third_party/WebKit/Source/web/SuspendableScriptExecutor.cpp

Status: Fixed (was: Started)
Marking this fixed so I can request a merge after the patch has settled a bit.

Comment 28 by phistuck@gmail.com, Jan 25 2017

#26 - does this basically unthrottle any DOM event and not just IndexedDB?
If so, does that basically revert an entire intended change?
#28: No, we just unthrottle things going into DOMWindowEventQueue, which is mostly IndexedDB and a small number of other miscellaneous events. According to the spec we could add a separate event queue just for IndexedDB and only unthrottle that, but unfortunately the code isn't really set up to do that at the moment.
Thanks for the update

I'm a VP of Engineering at Salesforce representing the Lightning Experience. This issue has severely impacted the Salesforce user base as many of our customers are complaining about this very problem and its impact to their business. How can this be accelerated and deployed to the current Chrome version? How quickly can this be done?
Labels: Merge-Request-56 ReleaseBlock-Stable
Requesting a merge to M56 to fix this fairly high impact regression.

#30: Assuming we get the green light, the fix should go out with Chrome M56 around January 31st.
Project Member

Comment 32 by sheriffbot@chromium.org, Jan 25 2017

Labels: -Merge-Request-56 Merge-Review-56 Hotlist-Merge-Review
This bug requires manual review: Only 5 days from stable, we might already have a stable candidate build
Please contact the milestone owner if you have questions.
Owners: amineer@(clank), cmasso@(bling), gkihumba@(cros), bustamante@(desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Labels: M-56 Merge-Request-57
Ah, looks like M57 already branched. Let's get this in there as well.
M56 Desktop owner here - this looks good to me for merging, but we need to let it bake in canary for 24 hours to make sure there aren't any unintended regressions.  I'll approve the merge tomorrow if there aren't any issues.

Then it will be included in the 1/31 stable release.
Cc: pbomm...@chromium.org anan...@chromium.org ligim...@chromium.org
Requesting a postmortem for this as it regressed on M55 stable. 
Cc: bustamante@chromium.org
Project Member

Comment 37 by sheriffbot@chromium.org, Jan 26 2017

Labels: -Merge-Request-57 Hotlist-Merge-Approved Merge-Approved-57
Your change meets the bar and is auto-approved for M57. Please go ahead and merge the CL to branch 2987 manually. Please contact milestone owner if you have questions.
Owners: amineer@(clank), cmasso@(bling), ketakid@(cros), govind@(desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
#35: We are preparing a postmortem together with the Docs team.
Project Member

Comment 39 by bugdroid1@chromium.org, Jan 26 2017

Labels: -merge-approved-57 merge-merged-2987
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/e695fd410c82bc1909531ec7cbbfc03f9d6a5d0e

commit e695fd410c82bc1909531ec7cbbfc03f9d6a5d0e
Author: Sami Kyostila <skyostil@chromium.org>
Date: Thu Jan 26 18:40:04 2017

scheduler: Make DOM window event tasks unthrottled

This patch avoids throttling events that are routed via
DOMWindowEventQueue. This affects a handful of window-related events,
the most important ones being IndexedDB events. It turns out many web
pages (e.g., Google Docs) rely on IndexedDB transactions to be
relatively fast and start encountering errors if they are throttled too
aggressively while a tab is in the background.

BUG= 675372 

Review-Url: https://codereview.chromium.org/2651933002
Cr-Commit-Position: refs/heads/master@{#446000}
(cherry picked from commit 05a13309259dd16f38eef41adc7cce55e3d15a05)

Review-Url: https://codereview.chromium.org/2658583006 .
Cr-Commit-Position: refs/branch-heads/2987@{#111}
Cr-Branched-From: ad51088c0e8776e8dcd963dbe752c4035ba6dab6-refs/heads/master@{#444943}

[modify] https://crrev.com/e695fd410c82bc1909531ec7cbbfc03f9d6a5d0e/third_party/WebKit/Source/core/events/DOMWindowEventQueue.cpp
[modify] https://crrev.com/e695fd410c82bc1909531ec7cbbfc03f9d6a5d0e/third_party/WebKit/Source/core/frame/DOMTimer.cpp
[modify] https://crrev.com/e695fd410c82bc1909531ec7cbbfc03f9d6a5d0e/third_party/WebKit/Source/core/frame/LocalDOMWindow.cpp
[modify] https://crrev.com/e695fd410c82bc1909531ec7cbbfc03f9d6a5d0e/third_party/WebKit/Source/core/frame/SuspendableTimer.cpp
[modify] https://crrev.com/e695fd410c82bc1909531ec7cbbfc03f9d6a5d0e/third_party/WebKit/Source/core/frame/SuspendableTimer.h
[modify] https://crrev.com/e695fd410c82bc1909531ec7cbbfc03f9d6a5d0e/third_party/WebKit/Source/web/SuspendableScriptExecutor.cpp

bustamante@: Okay to merge to M56 now?
Labels: -Merge-Review-56 Merge-Approved-56
Yeah LGTM, approving for merge
Project Member

Comment 42 by bugdroid1@chromium.org, Jan 27 2017

Labels: -merge-approved-56 merge-merged-2924
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/15a15f86d18a09795ac438cec86be99f9b2c6334

commit 15a15f86d18a09795ac438cec86be99f9b2c6334
Author: Sami Kyostila <skyostil@chromium.org>
Date: Fri Jan 27 20:15:48 2017

scheduler: Make DOM window event tasks unthrottled

This patch avoids throttling events that are routed via
DOMWindowEventQueue. This affects a handful of window-related events,
the most important ones being IndexedDB events. It turns out many web
pages (e.g., Google Docs) rely on IndexedDB transactions to be
relatively fast and start encountering errors if they are throttled too
aggressively while a tab is in the background.

BUG= 675372 

Review-Url: https://codereview.chromium.org/2651933002
Cr-Commit-Position: refs/heads/master@{#446000}
(cherry picked from commit 05a13309259dd16f38eef41adc7cce55e3d15a05)

Review-Url: https://codereview.chromium.org/2658573011 .
Cr-Commit-Position: refs/branch-heads/2924@{#879}
Cr-Branched-From: 3a87aecc31cd1ffe751dd72c04e5a96a1fc8108a-refs/heads/master@{#433059}

[modify] https://crrev.com/15a15f86d18a09795ac438cec86be99f9b2c6334/third_party/WebKit/Source/core/events/DOMWindowEventQueue.cpp
[modify] https://crrev.com/15a15f86d18a09795ac438cec86be99f9b2c6334/third_party/WebKit/Source/core/frame/DOMTimer.cpp
[modify] https://crrev.com/15a15f86d18a09795ac438cec86be99f9b2c6334/third_party/WebKit/Source/core/frame/LocalDOMWindow.cpp
[modify] https://crrev.com/15a15f86d18a09795ac438cec86be99f9b2c6334/third_party/WebKit/Source/core/frame/SuspendableTimer.cpp
[modify] https://crrev.com/15a15f86d18a09795ac438cec86be99f9b2c6334/third_party/WebKit/Source/core/frame/SuspendableTimer.h
[modify] https://crrev.com/15a15f86d18a09795ac438cec86be99f9b2c6334/third_party/WebKit/Source/web/SuspendableScriptExecutor.cpp

Project Member

Comment 43 by bugdroid1@chromium.org, Jan 27 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c25072ab9773a783a0f25c00921d3f5108181579

commit c25072ab9773a783a0f25c00921d3f5108181579
Author: Sami Kyostila <skyostil@chromium.org>
Date: Fri Jan 27 22:20:27 2017

Fix a build failure caused by an earlier cherry pick

Cherry-pick: https://codereview.chromium.org/2658573011

BUG= 675372 ,686244
TBR=haraken@chromium.org

Review-Url: https://codereview.chromium.org/2663553002 .
Cr-Commit-Position: refs/branch-heads/2924@{#880}
Cr-Branched-From: 3a87aecc31cd1ffe751dd72c04e5a96a1fc8108a-refs/heads/master@{#433059}

[modify] https://crrev.com/c25072ab9773a783a0f25c00921d3f5108181579/third_party/WebKit/Source/core/frame/SuspendableTimer.h

Which version of 56 is this cherrypicked into?

56 seems to be rolling out in stable today - is it meant to be in that version, or some future update?
A future update this week, the 56 currently in stable is the early version pushed out last week.
Is there an ETA when this fix will be available (in Chrome 56)?  The chrome project page says version 56 estimated availability the week of the 31st.  Will the initial rollout of 56 include this fix?

We are trying to see if this fix solves the performance issues we are experiencing.
The current ETA is this Wednesday 2/1, instead of the 31st as originally planned, so that we can get in some other fixes as well.
Since this has been merged into the M56 branch already, the Chrome OS stable release will have the fix. 
Labels: TE-Verified-M56 TE-Verified-56.0.2924.87
Tested the issue on Windows-10 using chrome latest M56-56.0.2924.87 by following steps mentioned in the original comment. Observed that The background one finishes time as expected. Hence adding TE-Verified label.
Please find the screen shot for reference.

Thank you!
675372.PNG
556 KB View Download
How can I download a specific version of Chrome > 56.0.2924.81 so that I can verify this bug locally?

#50 - here - https://commondatastorage.googleapis.com/chromium-browser-snapshots/index.html
Take one of the highest build number for your platform.

Or just install Chrome canary (it installs side by side with stable Chrome and uses a separate, clean user profile) -
https://www.google.com/intl/en/chrome/browser/canary.html
What is the ETA for a build of Chrome with this fix ( > 56.0.2924.81 ) released in the stable channel?
#52 and everyone - the fix has shipped, update your Chrome to Chrome 56.0.2924.87.

https://chromium.googlesource.com/chromium/src/+log/56.0.2924.76..56.0.2924.87?pretty=fuller&n=10000
Thanks! Fix is working great for us.
#34: I've posted an (internal) postmortem: https://groups.google.com/a/google.com/forum/#!topic/chrome-team/SD0eeMnip9Q
Cc: ranjitkan@chromium.org
Labels: TE-Verified-57.0.2987.40 TE-Verified-M57
Rechecked the issue on M57 chrome version 57.0.2987.40 on Windows 10 using the extension provided in comment#3 and merge is working as intended in M57 as well.

Adding TE-verified labels.

Thanks.!


Sign in to add a comment