Make reprioritization IPC batch rather than per-request.

Make reprioritization IPC batch rather than per-request.

This is to reduce IPC overhead and quadratic searching through requests
on batch reprioritizations (primarily image reprioritizations, but soon
also reprioritization due to parsing stage).

BUG=600839

[Randy Smith (Not in Mondays), 2016-12-05 16:03:36]

Description was changed from

==========
Make reprioritization IPC batch rather than per-request.

This is to reduce IPC overhead and quadratic searching through requests
on batch reprioritizations (primarily image reprioritizations, but soon
also reprioritization due to parsing stage).

BUG=
==========

to

==========
Make reprioritization IPC batch rather than per-request.

This is to reduce IPC overhead and quadratic searching through requests
on batch reprioritizations (primarily image reprioritizations, but soon
also reprioritization due to parsing stage).

BUG= 600839
==========

[Randy Smith (Not in Mondays), 2016-12-05 16:03:36]

rdsmith@chromium.org changed reviewers:
+ csharrison@chromium.org, kinuko@chromium.org

[Randy Smith (Not in Mondays), 2016-12-05 16:03:36]

rdsmith@chromium.org changed required reviewers:
+ csharrison@chromium.org, kinuko@chromium.org

[Randy Smith (Not in Mondays), 2016-12-05 16:15:12]

Kinuko: Could you take a look at this CL and give me high level comments?  (It's
still in early stages; details at bottom of message.)  

The goal of the CL is twofold.  The first is performance: Charles tells me that
loading http://cr.kungfoo.net/loading/1000spacers/ the repeated (quadratic)
search through requests in ResourceScheduler for what to start next takes 5% of
the CPU, so I'm hopeful that this CL will reduce CPU usage even for less
pathological web sites.  The second is that I'm planning to implement the
renderer size reprioritization as described in
https://docs.google.com/document/d/1p-XGz-BFFehs5j2olbkDkjrXzxzT3n45PVcLSZzOV...,
and that's going to result in another source of batch reprioritizations, so I'd
like to take care of the batching problem first as I suspect otherwise I'm going
to regress performance. 

Charles: I'll want you do the c/b/l side of the review (in addition to whatever
else you'd like to review), but I don't think there are architectural level
questions on that side of the review, so you're welcome to review or not right
now as you wish.  I'll give you a second heads up when I'm ready for detailed
review.

(Things I haven't done but plan to: Tests, explore simplifying typedefs,
evaluate teardown & request/child kill races, evaluate naming, remove duplicate
code.)

[Charlie Harrison, 2016-12-05 17:39:05]

Thanks. Let me wait for high level thoughts from kinuko@ before reviewing this.

[kinuko, 2016-12-06 00:34:33]

Thanks for asking for high-level comment before you go too far.  Depending on
situations I could live with this change but let me start with some due
diligence.

https://codereview.chromium.org/2552703003/diff/20001/content/child/resource_...
File content/child/resource_dispatcher.cc (right):

https://codereview.chromium.org/2552703003/diff/20001/content/child/resource_...
content/child/resource_dispatcher.cc:576: request_id, new_priority,
intra_priority_value));
This change itself makes sense, while I'm hoping not to introduce too many local
batching optimizations like this at this layer (because-- maybe you already
know, but-- 1) this content code is supposed to go away at sometime next eyar,
and 2) we have so many places we can add this kind of code and if we do it at
every place things would be messy).  Will you be able to quickly see how
impactful this change could be?  How much this could block your work?

[Charlie Harrison, 2016-12-06 01:21:27]

https://codereview.chromium.org/2552703003/diff/20001/content/child/resource_...
File content/child/resource_dispatcher.cc (right):

https://codereview.chromium.org/2552703003/diff/20001/content/child/resource_...
content/child/resource_dispatcher.cc:576: request_id, new_priority,
intra_priority_value));
On 2016/12/06 00:34:33, kinuko wrote:
> This change itself makes sense, while I'm hoping not to introduce too many
local
> batching optimizations like this at this layer (because-- maybe you already
> know, but-- 1) this content code is supposed to go away at sometime next eyar,
> and 2) we have so many places we can add this kind of code and if we do it at
> every place things would be messy).  Will you be able to quickly see how
> impactful this change could be?  How much this could block your work?

We may need to do some lab tests to see how bad the situation is when the
throttled code lands behind a flag. The performance may not be bad enough for us
to block on this (in particular, the new network throttling code has no
quadratic behavior).

My main worry is that, high level, we are moving the single IPC of "hit body" to
a batch of reprioritization IPCs. Today, "hit body" causes reprioritization in
the ResourceScheduler, but in the future, each request will individually
reprioritize when the body tag is hit, causing an IPC storm of basically every
request made in <head>.

[Randy Smith (Not in Mondays), 2016-12-07 17:49:21]

So I'd lay out the issues as follows:

* Current performance gain from this change:  I used Charles' example of
http://cr.kungfoo.net/loading/1000spacers/ and the time reprioritization
(presumably images) takes on the IO thread drops from ~60ms to ~3ms (Total IO
Thread CPU time is ~430ms).  This is a highly contrived benchmark, and I
wouldn't put a lot of weight on it (it's a metric ton of images, loaded from the
cache so we're getting no overlap with network latency) but it does show the
potential current upside.

* Relationship to my work: The issue here is that I'm planning to run a field
trial to evaluate the splitting of the body tag reprioritization into network
and blink (see
https://docs.google.com/document/d/1p-XGz-BFFehs5j2olbkDkjrXzxzT3n45PVcLSZzOV...
; this is part of the "Switch ResourceScheduler to, instead of doing the current
pre-body throttling, passing those requests through (respecting the global
throttle) but changing the priority to THROTTLED. " item.  I expect that change
will have some performance issues, both positive and negative, on its own (just
because of the changes in priority handling).  If we don't batch prioritization
before I start that field trial, it's going to be laboring under a known
regression (because I'll be sending a lot more IPCs in the enabled trial), which
will make it harder for me to figure out if the change is ok or not.  

Having said that, I suspect if the IPCs cause a major problem I can see that in
local testing (pre-field trial) and can revisit this CL at that point.  So if
you'd like me to keep this CL on ice until I do local testing of my next change,
I'm willing--it would just make my life simpler to push it through now. 

WDYT?

[kinuko, 2016-12-08 06:49:11]

Thanks for giving more contexts, Randy and Charles! 

On 2016/12/07 17:49:21, Randy Smith - Not in Mondays wrote:
> So I'd lay out the issues as follows:
> 
> * Current performance gain from this change:  I used Charles' example of
> http://cr.kungfoo.net/loading/1000spacers/ and the time reprioritization
> (presumably images) takes on the IO thread drops from ~60ms to ~3ms (Total IO
> Thread CPU time is ~430ms).  This is a highly contrived benchmark, and I
> wouldn't put a lot of weight on it (it's a metric ton of images, loaded from
the
> cache so we're getting no overlap with network latency) but it does show the
> potential current upside.
> 
> * Relationship to my work: The issue here is that I'm planning to run a field
> trial to evaluate the splitting of the body tag reprioritization into network
> and blink (see
>
https://docs.google.com/document/d/1p-XGz-BFFehs5j2olbkDkjrXzxzT3n45PVcLSZzOV...
> ; this is part of the "Switch ResourceScheduler to, instead of doing the
current
> pre-body throttling, passing those requests through (respecting the global
> throttle) but changing the priority to THROTTLED. " item.  I expect that
change
> will have some performance issues, both positive and negative, on its own
(just
> because of the changes in priority handling).  If we don't batch
prioritization
> before I start that field trial, it's going to be laboring under a known
> regression (because I'll be sending a lot more IPCs in the enabled trial),
which
> will make it harder for me to figure out if the change is ok or not.  

I see. If this is the case I think I'm fine with landing this, but would like us
to leave a very explicit TODO comment to revisit if we could generalize this
kind of batching (you could add my ldap there).  I've just filed a meta bug
(crbug.com/672370) to track this kind of issues.

> Having said that, I suspect if the IPCs cause a major problem I can see that
in
> local testing (pre-field trial) and can revisit this CL at that point.  So if
> you'd like me to keep this CL on ice until I do local testing of my next
change,
> I'm willing--it would just make my life simpler to push it through now. 

This also sounds good :) I plan to do some investigation around IPC this month
(while everyone's quiet :)) so if you could hold off a bit it could work better
for me.

> WDYT?

[Randy Smith (Not in Mondays), 2016-12-08 15:19:23]

On 2016/12/08 06:49:11, kinuko wrote:
> Thanks for giving more contexts, Randy and Charles! 
> 
> On 2016/12/07 17:49:21, Randy Smith - Not in Mondays wrote:
> > So I'd lay out the issues as follows:
> > 
> > * Current performance gain from this change:  I used Charles' example of
> > http://cr.kungfoo.net/loading/1000spacers/ and the time reprioritization
> > (presumably images) takes on the IO thread drops from ~60ms to ~3ms (Total
IO
> > Thread CPU time is ~430ms).  This is a highly contrived benchmark, and I
> > wouldn't put a lot of weight on it (it's a metric ton of images, loaded from
> the
> > cache so we're getting no overlap with network latency) but it does show the
> > potential current upside.
> > 
> > * Relationship to my work: The issue here is that I'm planning to run a
field
> > trial to evaluate the splitting of the body tag reprioritization into
network
> > and blink (see
> >
>
https://docs.google.com/document/d/1p-XGz-BFFehs5j2olbkDkjrXzxzT3n45PVcLSZzOV...
> > ; this is part of the "Switch ResourceScheduler to, instead of doing the
> current
> > pre-body throttling, passing those requests through (respecting the global
> > throttle) but changing the priority to THROTTLED. " item.  I expect that
> change
> > will have some performance issues, both positive and negative, on its own
> (just
> > because of the changes in priority handling).  If we don't batch
> prioritization
> > before I start that field trial, it's going to be laboring under a known
> > regression (because I'll be sending a lot more IPCs in the enabled trial),
> which
> > will make it harder for me to figure out if the change is ok or not.  
> 
> I see. If this is the case I think I'm fine with landing this, but would like
us
> to leave a very explicit TODO comment to revisit if we could generalize this
> kind of batching (you could add my ldap there).  I've just filed a meta bug
> (crbug.com/672370) to track this kind of issues.
> 
> > Having said that, I suspect if the IPCs cause a major problem I can see that
> in
> > local testing (pre-field trial) and can revisit this CL at that point.  So
if
> > you'd like me to keep this CL on ice until I do local testing of my next
> change,
> > I'm willing--it would just make my life simpler to push it through now. 
> 
> This also sounds good :) I plan to do some investigation around IPC this month
> (while everyone's quiet :)) so if you could hold off a bit it could work
better
> for me.
> 
> > WDYT?

I think given what you're saying I'll hold off for a bit, at least until I have
the next CL working and have done local tests.  I'll ping you back at that
point, and we'll decide what the right next step is, before I run the field
trial.  Please let me know what you find out in your IPC investigation :-} :-|.

Thanks!

[kinuko, 2016-12-09 03:01:29]

On 2016/12/08 15:19:23, Randy Smith - Not in Mondays wrote:
> On 2016/12/08 06:49:11, kinuko wrote:
> > Thanks for giving more contexts, Randy and Charles! 
> > 
> > On 2016/12/07 17:49:21, Randy Smith - Not in Mondays wrote:
> > > So I'd lay out the issues as follows:
> > > 
> > > * Current performance gain from this change:  I used Charles' example of
> > > http://cr.kungfoo.net/loading/1000spacers/ and the time reprioritization
> > > (presumably images) takes on the IO thread drops from ~60ms to ~3ms (Total
> IO
> > > Thread CPU time is ~430ms).  This is a highly contrived benchmark, and I
> > > wouldn't put a lot of weight on it (it's a metric ton of images, loaded
from
> > the
> > > cache so we're getting no overlap with network latency) but it does show
the
> > > potential current upside.
> > > 
> > > * Relationship to my work: The issue here is that I'm planning to run a
> field
> > > trial to evaluate the splitting of the body tag reprioritization into
> network
> > > and blink (see
> > >
> >
>
https://docs.google.com/document/d/1p-XGz-BFFehs5j2olbkDkjrXzxzT3n45PVcLSZzOV...
> > > ; this is part of the "Switch ResourceScheduler to, instead of doing the
> > current
> > > pre-body throttling, passing those requests through (respecting the global
> > > throttle) but changing the priority to THROTTLED. " item.  I expect that
> > change
> > > will have some performance issues, both positive and negative, on its own
> > (just
> > > because of the changes in priority handling).  If we don't batch
> > prioritization
> > > before I start that field trial, it's going to be laboring under a known
> > > regression (because I'll be sending a lot more IPCs in the enabled trial),
> > which
> > > will make it harder for me to figure out if the change is ok or not.  
> > 
> > I see. If this is the case I think I'm fine with landing this, but would
like
> us
> > to leave a very explicit TODO comment to revisit if we could generalize this
> > kind of batching (you could add my ldap there).  I've just filed a meta bug
> > (crbug.com/672370) to track this kind of issues.
> > 
> > > Having said that, I suspect if the IPCs cause a major problem I can see
that
> > in
> > > local testing (pre-field trial) and can revisit this CL at that point.  So
> if
> > > you'd like me to keep this CL on ice until I do local testing of my next
> > change,
> > > I'm willing--it would just make my life simpler to push it through now. 
> > 
> > This also sounds good :) I plan to do some investigation around IPC this
month
> > (while everyone's quiet :)) so if you could hold off a bit it could work
> better
> > for me.
> > 
> > > WDYT?
> 
> I think given what you're saying I'll hold off for a bit, at least until I
have
> the next CL working and have done local tests.  I'll ping you back at that
> point, and we'll decide what the right next step is, before I run the field
> trial.  Please let me know what you find out in your IPC investigation :-}
:-|.
> 
> Thanks!

SGTM. Please feel free to ping me again anytime!

[kinuko, 2016-12-09 10:52:44]

On 2016/12/09 03:01:29, kinuko wrote:
> On 2016/12/08 15:19:23, Randy Smith - Not in Mondays wrote:
> > On 2016/12/08 06:49:11, kinuko wrote:
> > > Thanks for giving more contexts, Randy and Charles! 
> > > 
> > > On 2016/12/07 17:49:21, Randy Smith - Not in Mondays wrote:
> > > > So I'd lay out the issues as follows:
> > > > 
> > > > * Current performance gain from this change:  I used Charles' example of
> > > > http://cr.kungfoo.net/loading/1000spacers/ and the time reprioritization
> > > > (presumably images) takes on the IO thread drops from ~60ms to ~3ms
(Total
> > IO
> > > > Thread CPU time is ~430ms).  This is a highly contrived benchmark, and I
> > > > wouldn't put a lot of weight on it (it's a metric ton of images, loaded
> from
> > > the
> > > > cache so we're getting no overlap with network latency) but it does show
> the
> > > > potential current upside.
> > > > 
> > > > * Relationship to my work: The issue here is that I'm planning to run a
> > field
> > > > trial to evaluate the splitting of the body tag reprioritization into
> > network
> > > > and blink (see
> > > >
> > >
> >
>
https://docs.google.com/document/d/1p-XGz-BFFehs5j2olbkDkjrXzxzT3n45PVcLSZzOV...
> > > > ; this is part of the "Switch ResourceScheduler to, instead of doing the
> > > current
> > > > pre-body throttling, passing those requests through (respecting the
global
> > > > throttle) but changing the priority to THROTTLED. " item.  I expect that
> > > change
> > > > will have some performance issues, both positive and negative, on its
own
> > > (just
> > > > because of the changes in priority handling).  If we don't batch
> > > prioritization
> > > > before I start that field trial, it's going to be laboring under a known
> > > > regression (because I'll be sending a lot more IPCs in the enabled
trial),
> > > which
> > > > will make it harder for me to figure out if the change is ok or not.  
> > > 
> > > I see. If this is the case I think I'm fine with landing this, but would
> like
> > us
> > > to leave a very explicit TODO comment to revisit if we could generalize
this
> > > kind of batching (you could add my ldap there).  I've just filed a meta
bug
> > > (crbug.com/672370) to track this kind of issues.
> > > 
> > > > Having said that, I suspect if the IPCs cause a major problem I can see
> that
> > > in
> > > > local testing (pre-field trial) and can revisit this CL at that point. 
So
> > if
> > > > you'd like me to keep this CL on ice until I do local testing of my next
> > > change,
> > > > I'm willing--it would just make my life simpler to push it through now. 
> > > 
> > > This also sounds good :) I plan to do some investigation around IPC this
> month
> > > (while everyone's quiet :)) so if you could hold off a bit it could work
> > better
> > > for me.
> > > 
> > > > WDYT?
> > 
> > I think given what you're saying I'll hold off for a bit, at least until I
> have
> > the next CL working and have done local tests.  I'll ping you back at that
> > point, and we'll decide what the right next step is, before I run the field
> > trial.  Please let me know what you find out in your IPC investigation :-}
> :-|.
> > 
> > Thanks!
> 
> SGTM. Please feel free to ping me again anytime!

By the way-- let me make sure, you're worrying about the case for
ResourceFetcher::updateAllImageResourcePriorities() where bunch of
didChangePriorities requests could be created in a tight loop, am I right?

[Randy Smith (Not in Mondays), 2016-12-09 13:44:08]

On 2016/12/09 10:52:44, kinuko wrote:
> On 2016/12/09 03:01:29, kinuko wrote:
> > On 2016/12/08 15:19:23, Randy Smith - Not in Mondays wrote:
> > > On 2016/12/08 06:49:11, kinuko wrote:
> > > > Thanks for giving more contexts, Randy and Charles! 
> > > > 
> > > > On 2016/12/07 17:49:21, Randy Smith - Not in Mondays wrote:
> > > > > So I'd lay out the issues as follows:
> > > > > 
> > > > > * Current performance gain from this change:  I used Charles' example
of
> > > > > http://cr.kungfoo.net/loading/1000spacers/ and the time
reprioritization
> > > > > (presumably images) takes on the IO thread drops from ~60ms to ~3ms
> (Total
> > > IO
> > > > > Thread CPU time is ~430ms).  This is a highly contrived benchmark, and
I
> > > > > wouldn't put a lot of weight on it (it's a metric ton of images,
loaded
> > from
> > > > the
> > > > > cache so we're getting no overlap with network latency) but it does
show
> > the
> > > > > potential current upside.
> > > > > 
> > > > > * Relationship to my work: The issue here is that I'm planning to run
a
> > > field
> > > > > trial to evaluate the splitting of the body tag reprioritization into
> > > network
> > > > > and blink (see
> > > > >
> > > >
> > >
> >
>
https://docs.google.com/document/d/1p-XGz-BFFehs5j2olbkDkjrXzxzT3n45PVcLSZzOV...
> > > > > ; this is part of the "Switch ResourceScheduler to, instead of doing
the
> > > > current
> > > > > pre-body throttling, passing those requests through (respecting the
> global
> > > > > throttle) but changing the priority to THROTTLED. " item.  I expect
that
> > > > change
> > > > > will have some performance issues, both positive and negative, on its
> own
> > > > (just
> > > > > because of the changes in priority handling).  If we don't batch
> > > > prioritization
> > > > > before I start that field trial, it's going to be laboring under a
known
> > > > > regression (because I'll be sending a lot more IPCs in the enabled
> trial),
> > > > which
> > > > > will make it harder for me to figure out if the change is ok or not.  
> > > > 
> > > > I see. If this is the case I think I'm fine with landing this, but would
> > like
> > > us
> > > > to leave a very explicit TODO comment to revisit if we could generalize
> this
> > > > kind of batching (you could add my ldap there).  I've just filed a meta
> bug
> > > > (crbug.com/672370) to track this kind of issues.
> > > > 
> > > > > Having said that, I suspect if the IPCs cause a major problem I can
see
> > that
> > > > in
> > > > > local testing (pre-field trial) and can revisit this CL at that point.

> So
> > > if
> > > > > you'd like me to keep this CL on ice until I do local testing of my
next
> > > > change,
> > > > > I'm willing--it would just make my life simpler to push it through
now. 
> > > > 
> > > > This also sounds good :) I plan to do some investigation around IPC this
> > month
> > > > (while everyone's quiet :)) so if you could hold off a bit it could work
> > > better
> > > > for me.
> > > > 
> > > > > WDYT?
> > > 
> > > I think given what you're saying I'll hold off for a bit, at least until I
> > have
> > > the next CL working and have done local tests.  I'll ping you back at that
> > > point, and we'll decide what the right next step is, before I run the
field
> > > trial.  Please let me know what you find out in your IPC investigation :-}
> > :-|.
> > > 
> > > Thanks!
> > 
> > SGTM. Please feel free to ping me again anytime!
> 
> By the way-- let me make sure, you're worrying about the case for
> ResourceFetcher::updateAllImageResourcePriorities() where bunch of
> didChangePriorities requests could be created in a tight loop, am I right?

Yes, that's the primary existing use case this CL targets.  I'm amused that that
isn't clear from the CL given that that's the function I traced code paths from,
but that's just because after hitting my head against the problem of plumbing
down a batch message from that level, I threw up my hands and just coded the
queue&post solution at the ResourceDispatcher.

[kinuko, 2016-12-09 17:04:43]

On 2016/12/09 13:44:08, Randy Smith - Not in Mondays wrote:
> On 2016/12/09 10:52:44, kinuko wrote:
> > On 2016/12/09 03:01:29, kinuko wrote:
> > > On 2016/12/08 15:19:23, Randy Smith - Not in Mondays wrote:
> > > > On 2016/12/08 06:49:11, kinuko wrote:
> > > > > Thanks for giving more contexts, Randy and Charles! 
> > > > > 
> > > > > On 2016/12/07 17:49:21, Randy Smith - Not in Mondays wrote:
> > > > > > So I'd lay out the issues as follows:
> > > > > > 
> > > > > > * Current performance gain from this change:  I used Charles'
example
> of
> > > > > > http://cr.kungfoo.net/loading/1000spacers/ and the time
> reprioritization
> > > > > > (presumably images) takes on the IO thread drops from ~60ms to ~3ms
> > (Total
> > > > IO
> > > > > > Thread CPU time is ~430ms).  This is a highly contrived benchmark,
and
> I
> > > > > > wouldn't put a lot of weight on it (it's a metric ton of images,
> loaded
> > > from
> > > > > the
> > > > > > cache so we're getting no overlap with network latency) but it does
> show
> > > the
> > > > > > potential current upside.
> > > > > > 
> > > > > > * Relationship to my work: The issue here is that I'm planning to
run
> a
> > > > field
> > > > > > trial to evaluate the splitting of the body tag reprioritization
into
> > > > network
> > > > > > and blink (see
> > > > > >
> > > > >
> > > >
> > >
> >
>
https://docs.google.com/document/d/1p-XGz-BFFehs5j2olbkDkjrXzxzT3n45PVcLSZzOV...
> > > > > > ; this is part of the "Switch ResourceScheduler to, instead of doing
> the
> > > > > current
> > > > > > pre-body throttling, passing those requests through (respecting the
> > global
> > > > > > throttle) but changing the priority to THROTTLED. " item.  I expect
> that
> > > > > change
> > > > > > will have some performance issues, both positive and negative, on
its
> > own
> > > > > (just
> > > > > > because of the changes in priority handling).  If we don't batch
> > > > > prioritization
> > > > > > before I start that field trial, it's going to be laboring under a
> known
> > > > > > regression (because I'll be sending a lot more IPCs in the enabled
> > trial),
> > > > > which
> > > > > > will make it harder for me to figure out if the change is ok or not.
 
> > > > > 
> > > > > I see. If this is the case I think I'm fine with landing this, but
would
> > > like
> > > > us
> > > > > to leave a very explicit TODO comment to revisit if we could
generalize
> > this
> > > > > kind of batching (you could add my ldap there).  I've just filed a
meta
> > bug
> > > > > (crbug.com/672370) to track this kind of issues.
> > > > > 
> > > > > > Having said that, I suspect if the IPCs cause a major problem I can
> see
> > > that
> > > > > in
> > > > > > local testing (pre-field trial) and can revisit this CL at that
point.
> 
> > So
> > > > if
> > > > > > you'd like me to keep this CL on ice until I do local testing of my
> next
> > > > > change,
> > > > > > I'm willing--it would just make my life simpler to push it through
> now. 
> > > > > 
> > > > > This also sounds good :) I plan to do some investigation around IPC
this
> > > month
> > > > > (while everyone's quiet :)) so if you could hold off a bit it could
work
> > > > better
> > > > > for me.
> > > > > 
> > > > > > WDYT?
> > > > 
> > > > I think given what you're saying I'll hold off for a bit, at least until
I
> > > have
> > > > the next CL working and have done local tests.  I'll ping you back at
that
> > > > point, and we'll decide what the right next step is, before I run the
> field
> > > > trial.  Please let me know what you find out in your IPC investigation
:-}
> > > :-|.
> > > > 
> > > > Thanks!
> > > 
> > > SGTM. Please feel free to ping me again anytime!
> > 
> > By the way-- let me make sure, you're worrying about the case for
> > ResourceFetcher::updateAllImageResourcePriorities() where bunch of
> > didChangePriorities requests could be created in a tight loop, am I right?
> 
> Yes, that's the primary existing use case this CL targets.  I'm amused that
that
> isn't clear from the CL given that that's the function I traced code paths
from,
> but that's just because after hitting my head against the problem of plumbing
> down a batch message from that level, I threw up my hands and just coded the
> queue&post solution at the ResourceDispatcher.

Gotcha. Thanks for confirming!