Issue metadata
Sign in to add a comment
|
safe_browsing::BrowserURLLoaderThrottle should work on the UI thread |
||||||||||||||||||
Issue descriptionRight now the safe browsing database is checked on the IO thread. To improve performance and avoid extra/unnecessary thread hops (see parent bug), it would be great if we can make the database checks possible on the UI thread. I know it's not a bloom filter anymore, but it's something similar. Strawman: can we add lock-free ways of checking/updating the data structure?
,
Apr 2 2018
@Nathan: we don't have data yet, as we mostly only trust perf data from the field and can't run experiments until the parent bug and child bugs are all done. Here are some anecdotes: -for PlzNavigate and MojoNavigationResponse field trials, we saw regressions because of extra PostTasks in the same thread (renderer, browser). So we know that when the browser is busy with a navigation (and the renderer is running JS) every post task counts. removing one posttask removed a 5% regression at the 95% percentile -IO/UI thread: we also know that when the browser process is busy, posttasks between UI and IO thread can be very expensive. In the Maps Go investigations, we're seeing a 100ms delay posting a task between the IO and UI threads on the Go devices. -once we fix the parent bug, we can remove a lot of duplication of state between UI and IO threads. This is more of a code complexity task and not performance. To clarify one thing: we would only need to check SB in one thread at a time. i.e. if we move it to the UI thread, we wouldn't also need the database to be checked on the IO.
,
Apr 2 2018
A wholesale switch from IO to UI thread would probably require changing all the callers of SafeBrowsingDatabaseManager::{Check,Match}* functions. Some of thoe callers are already on the IO thread, though some are hopped from UI thread and so would be simpler.
It's probably best to get together on a whiteboard to think through the implications. We could add some locks, or move it all to UI thread, or something else. And again the amount of investment should correlate w/ our confidence in a perf win.
,
Apr 3 2018
,
Apr 3 2018
I would guess that one of the real reasons doing the checks on the IO thread is slow is that the IO thread is super busy doing expensive, CPU-heavy tasks even though it shouldn't be. We saw major speed-ups on Android when moving SB checks off onto the thread_scheduler. I wouldn't be terribly surprised if there were minimal regressions to moving SB checks to thread_scheduler on all platforms, especially if we ran them with high priority.
,
May 16 2018
btw we'll hold off on this until the network service ships. at that point, it'll be a lot easier (as the rest of the code will only be on UI).
,
Jan 10
Downgrading P2s that haven't been modified in more than 6 months, which have no component or owner. |
|||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||
Comment 1 by nparker@chromium.org
, Mar 30 2018