Expose MemoryCoordinator's global budget information.

Expose MemoryCoordinator's global budget information.

This information will be used by V8 to tweak its memory usage in real time.

BUG=

[chrisha, 2016-11-04 19:49:36]

chrisha@chromium.org changed reviewers:
+ bashi@chromium.org, haraken@chromium.org

[chrisha, 2016-11-04 19:49:37]

PTAL?

[haraken, 2016-11-05 12:52:22]

Can we add a test?

https://codereview.chromium.org/2479673002/diff/20001/content/browser/memory/...
File content/browser/memory/memory_coordinator_impl.cc (right):

https://codereview.chromium.org/2479673002/diff/20001/content/browser/memory/...
content/browser/memory/memory_coordinator_impl.cc:201: renderer_count =
new_renderers_back_to_normal_;

Shouldn't new_renderers_back_to_normal_ be always equal to
new_renderers_until_throttled_?

https://codereview.chromium.org/2479673002/diff/20001/content/browser/memory/...
content/browser/memory/memory_coordinator_impl.cc:239:
NotifyRemainingGlobalBudgetToChildren();

How frequently will this IPC be called?

https://codereview.chromium.org/2479673002/diff/20001/content/browser/memory/...
File content/browser/memory/memory_coordinator_impl.h (right):

https://codereview.chromium.org/2479673002/diff/20001/content/browser/memory/...
content/browser/memory/memory_coordinator_impl.h:72: MemoryState
CalculateNextState(int free_memory_until_critical_mb);

int => int64_t ?

https://codereview.chromium.org/2479673002/diff/20001/content/child/memory/ch...
File content/child/memory/child_memory_coordinator_impl.cc (right):

https://codereview.chromium.org/2479673002/diff/20001/content/child/memory/ch...
content/child/memory/child_memory_coordinator_impl.cc:39: return
ToBaseMemoryState(g_child_memory_coordinator->GetCurrentMemoryState());

g_child_memory_coordinator => GetInstance() ?

[bashi, 2016-11-07 00:16:37]

Thank you for adding this!

https://codereview.chromium.org/2479673002/diff/20001/base/memory/memory_coor...
File base/memory/memory_coordinator_proxy.h (right):

https://codereview.chromium.org/2479673002/diff/20001/base/memory/memory_coor...
base/memory/memory_coordinator_proxy.h:18: // is injected.
+1 :)

https://codereview.chromium.org/2479673002/diff/20001/base/memory/memory_coor...
base/memory/memory_coordinator_proxy.h:38: int64_t GetRemainingGlobalBudget()
const;
nit: we use int for MemoryMonitor::GetFreeMemoryUntilCriticalMB(). Should we use
int64_t everywhere?

https://codereview.chromium.org/2479673002/diff/20001/content/browser/memory/...
File content/browser/memory/memory_coordinator_impl.cc (right):

https://codereview.chromium.org/2479673002/diff/20001/content/browser/memory/...
content/browser/memory/memory_coordinator_impl.cc:201: renderer_count =
new_renderers_back_to_normal_;
On 2016/11/05 12:52:21, haraken wrote:
> 
> Shouldn't new_renderers_back_to_normal_ be always equal to
> new_renderers_until_throttled_?

We have different parameters for # of renderers to switch states as we discussed
before[1]. Basically back_to_normal > until_throttled

[1] https://codereview.chromium.org/2374343002#msg9

https://codereview.chromium.org/2479673002/diff/20001/content/browser/memory/...
content/browser/memory/memory_coordinator_impl.cc:237: if (prev_remaining !=
next_remaining) {
How likely does this condition become true? |free_memory_until_critical_mb|
isn't stable as far as I tested and I guess that |prev_remaining| and
|next_remaining| wouldn't be the same in most cases.

https://codereview.chromium.org/2479673002/diff/20001/content/browser/memory/...
content/browser/memory/memory_coordinator_impl.cc:239:
NotifyRemainingGlobalBudgetToChildren();
On 2016/11/05 12:52:21, haraken wrote:
> 
> How frequently will this IPC be called?

My guess is that it's almost the same as how frequently UpdateState() is called,
which means that every 5 seconds in general.

Instead of pushing budgets, how about adding a pull IPC method which a renderer
can ask the browser the current budget?

[haraken, 2016-11-07 01:44:40]

On 2016/11/07 00:16:37, bashi1 wrote:
> Thank you for adding this!
> 
>
https://codereview.chromium.org/2479673002/diff/20001/base/memory/memory_coor...
> File base/memory/memory_coordinator_proxy.h (right):
> 
>
https://codereview.chromium.org/2479673002/diff/20001/base/memory/memory_coor...
> base/memory/memory_coordinator_proxy.h:18: // is injected.
> +1 :)
> 
>
https://codereview.chromium.org/2479673002/diff/20001/base/memory/memory_coor...
> base/memory/memory_coordinator_proxy.h:38: int64_t GetRemainingGlobalBudget()
> const;
> nit: we use int for MemoryMonitor::GetFreeMemoryUntilCriticalMB(). Should we
use
> int64_t everywhere?
> 
>
https://codereview.chromium.org/2479673002/diff/20001/content/browser/memory/...
> File content/browser/memory/memory_coordinator_impl.cc (right):
> 
>
https://codereview.chromium.org/2479673002/diff/20001/content/browser/memory/...
> content/browser/memory/memory_coordinator_impl.cc:201: renderer_count =
> new_renderers_back_to_normal_;
> On 2016/11/05 12:52:21, haraken wrote:
> > 
> > Shouldn't new_renderers_back_to_normal_ be always equal to
> > new_renderers_until_throttled_?
> 
> We have different parameters for # of renderers to switch states as we
discussed
> before[1]. Basically back_to_normal > until_throttled
> 
> [1] https://codereview.chromium.org/2374343002#msg9
> 
>
https://codereview.chromium.org/2479673002/diff/20001/content/browser/memory/...
> content/browser/memory/memory_coordinator_impl.cc:237: if (prev_remaining !=
> next_remaining) {
> How likely does this condition become true? |free_memory_until_critical_mb|
> isn't stable as far as I tested and I guess that |prev_remaining| and
> |next_remaining| wouldn't be the same in most cases.
> 
>
https://codereview.chromium.org/2479673002/diff/20001/content/browser/memory/...
> content/browser/memory/memory_coordinator_impl.cc:239:
> NotifyRemainingGlobalBudgetToChildren();
> On 2016/11/05 12:52:21, haraken wrote:
> > 
> > How frequently will this IPC be called?
> 
> My guess is that it's almost the same as how frequently UpdateState() is
called,
> which means that every 5 seconds in general.
> 
> Instead of pushing budgets, how about adding a pull IPC method which a
renderer
> can ask the browser the current budget?

I'd prefer a well-controlled, push-based approach though. If we take a
pull-based approach, there is a risk that MCC requests a budget too frequently
and causes an IPC flood.

[bashi, 2016-11-07 02:02:22]

On 2016/11/07 01:44:40, haraken wrote:
> On 2016/11/07 00:16:37, bashi1 wrote:
> > Thank you for adding this!
> > 
> >
>
https://codereview.chromium.org/2479673002/diff/20001/base/memory/memory_coor...
> > File base/memory/memory_coordinator_proxy.h (right):
> > 
> >
>
https://codereview.chromium.org/2479673002/diff/20001/base/memory/memory_coor...
> > base/memory/memory_coordinator_proxy.h:18: // is injected.
> > +1 :)
> > 
> >
>
https://codereview.chromium.org/2479673002/diff/20001/base/memory/memory_coor...
> > base/memory/memory_coordinator_proxy.h:38: int64_t
GetRemainingGlobalBudget()
> > const;
> > nit: we use int for MemoryMonitor::GetFreeMemoryUntilCriticalMB(). Should we
> use
> > int64_t everywhere?
> > 
> >
>
https://codereview.chromium.org/2479673002/diff/20001/content/browser/memory/...
> > File content/browser/memory/memory_coordinator_impl.cc (right):
> > 
> >
>
https://codereview.chromium.org/2479673002/diff/20001/content/browser/memory/...
> > content/browser/memory/memory_coordinator_impl.cc:201: renderer_count =
> > new_renderers_back_to_normal_;
> > On 2016/11/05 12:52:21, haraken wrote:
> > > 
> > > Shouldn't new_renderers_back_to_normal_ be always equal to
> > > new_renderers_until_throttled_?
> > 
> > We have different parameters for # of renderers to switch states as we
> discussed
> > before[1]. Basically back_to_normal > until_throttled
> > 
> > [1] https://codereview.chromium.org/2374343002#msg9
> > 
> >
>
https://codereview.chromium.org/2479673002/diff/20001/content/browser/memory/...
> > content/browser/memory/memory_coordinator_impl.cc:237: if (prev_remaining !=
> > next_remaining) {
> > How likely does this condition become true? |free_memory_until_critical_mb|
> > isn't stable as far as I tested and I guess that |prev_remaining| and
> > |next_remaining| wouldn't be the same in most cases.
> > 
> >
>
https://codereview.chromium.org/2479673002/diff/20001/content/browser/memory/...
> > content/browser/memory/memory_coordinator_impl.cc:239:
> > NotifyRemainingGlobalBudgetToChildren();
> > On 2016/11/05 12:52:21, haraken wrote:
> > > 
> > > How frequently will this IPC be called?
> > 
> > My guess is that it's almost the same as how frequently UpdateState() is
> called,
> > which means that every 5 seconds in general.
> > 
> > Instead of pushing budgets, how about adding a pull IPC method which a
> renderer
> > can ask the browser the current budget?
> 
> I'd prefer a well-controlled, push-based approach though. If we take a
> pull-based approach, there is a risk that MCC requests a budget too frequently
> and causes an IPC flood.

We can suppress an IPC call for a certain period and return a cached value. My
concern is that running more periodic tasks may affect other performance (e.g.
battery). If renderers need to know budgets only when the state is changed,
pull-based API would make sense.

I'm fine with push-based approach. Just my 2 cents.

[haraken, 2016-11-07 02:07:57]

On 2016/11/07 02:02:22, bashi1 wrote:
> On 2016/11/07 01:44:40, haraken wrote:
> > On 2016/11/07 00:16:37, bashi1 wrote:
> > > Thank you for adding this!
> > > 
> > >
> >
>
https://codereview.chromium.org/2479673002/diff/20001/base/memory/memory_coor...
> > > File base/memory/memory_coordinator_proxy.h (right):
> > > 
> > >
> >
>
https://codereview.chromium.org/2479673002/diff/20001/base/memory/memory_coor...
> > > base/memory/memory_coordinator_proxy.h:18: // is injected.
> > > +1 :)
> > > 
> > >
> >
>
https://codereview.chromium.org/2479673002/diff/20001/base/memory/memory_coor...
> > > base/memory/memory_coordinator_proxy.h:38: int64_t
> GetRemainingGlobalBudget()
> > > const;
> > > nit: we use int for MemoryMonitor::GetFreeMemoryUntilCriticalMB(). Should
we
> > use
> > > int64_t everywhere?
> > > 
> > >
> >
>
https://codereview.chromium.org/2479673002/diff/20001/content/browser/memory/...
> > > File content/browser/memory/memory_coordinator_impl.cc (right):
> > > 
> > >
> >
>
https://codereview.chromium.org/2479673002/diff/20001/content/browser/memory/...
> > > content/browser/memory/memory_coordinator_impl.cc:201: renderer_count =
> > > new_renderers_back_to_normal_;
> > > On 2016/11/05 12:52:21, haraken wrote:
> > > > 
> > > > Shouldn't new_renderers_back_to_normal_ be always equal to
> > > > new_renderers_until_throttled_?
> > > 
> > > We have different parameters for # of renderers to switch states as we
> > discussed
> > > before[1]. Basically back_to_normal > until_throttled
> > > 
> > > [1] https://codereview.chromium.org/2374343002#msg9
> > > 
> > >
> >
>
https://codereview.chromium.org/2479673002/diff/20001/content/browser/memory/...
> > > content/browser/memory/memory_coordinator_impl.cc:237: if (prev_remaining
!=
> > > next_remaining) {
> > > How likely does this condition become true?
|free_memory_until_critical_mb|
> > > isn't stable as far as I tested and I guess that |prev_remaining| and
> > > |next_remaining| wouldn't be the same in most cases.
> > > 
> > >
> >
>
https://codereview.chromium.org/2479673002/diff/20001/content/browser/memory/...
> > > content/browser/memory/memory_coordinator_impl.cc:239:
> > > NotifyRemainingGlobalBudgetToChildren();
> > > On 2016/11/05 12:52:21, haraken wrote:
> > > > 
> > > > How frequently will this IPC be called?
> > > 
> > > My guess is that it's almost the same as how frequently UpdateState() is
> > called,
> > > which means that every 5 seconds in general.
> > > 
> > > Instead of pushing budgets, how about adding a pull IPC method which a
> > renderer
> > > can ask the browser the current budget?
> > 
> > I'd prefer a well-controlled, push-based approach though. If we take a
> > pull-based approach, there is a risk that MCC requests a budget too
frequently
> > and causes an IPC flood.
> 
> We can suppress an IPC call for a certain period and return a cached value. My
> concern is that running more periodic tasks may affect other performance (e.g.
> battery). If renderers need to know budgets only when the state is changed,
> pull-based API would make sense.
> 
> I'm fine with push-based approach. Just my 2 cents.

Yeah, agreed. I think:

- From the perspective of MCC, synchronous APIs would be easier to use than
asynchronous (callback-style) APIs.

- This means that we need to cache the budget on CMC. The cache doesn't
necessarily need to be strictly consistent (it's okay to be eventually
consistent).

I don't have any strong opinion about whether we should implement the cache by a
push-based approach or a pull-based approach.

[chrisha, 2016-11-07 19:33:42]

Thinking this over more, I agree that a "pull" makes more sense here. I'll
rewrite this.

https://codereview.chromium.org/2479673002/diff/20001/base/memory/memory_coor...
File base/memory/memory_coordinator_proxy.h (right):

https://codereview.chromium.org/2479673002/diff/20001/base/memory/memory_coor...
base/memory/memory_coordinator_proxy.h:18: // is injected.
On 2016/11/07 00:16:37, bashi1 wrote:
> +1 :)

I'll do this in a follow-up CL, as it's going to be a bit of a mess to do here.

https://codereview.chromium.org/2479673002/diff/20001/base/memory/memory_coor...
base/memory/memory_coordinator_proxy.h:38: int64_t GetRemainingGlobalBudget()
const;
On 2016/11/07 00:16:37, bashi1 wrote:
> nit: we use int for MemoryMonitor::GetFreeMemoryUntilCriticalMB(). Should we
use
> int64_t everywhere?

Well, an auto-scaled 'int' is sufficient for each platform, but Mojo has no such
thing, and forces me to use an int64_t in order to accommodate both platforms.
I'd prefer to go the other way and keep everything an int, except for the IPC
message itself?

https://codereview.chromium.org/2479673002/diff/20001/content/browser/memory/...
File content/browser/memory/memory_coordinator_impl.cc (right):

https://codereview.chromium.org/2479673002/diff/20001/content/browser/memory/...
content/browser/memory/memory_coordinator_impl.cc:201: renderer_count =
new_renderers_back_to_normal_;
On 2016/11/07 00:16:37, bashi1 wrote:
> On 2016/11/05 12:52:21, haraken wrote:
> > 
> > Shouldn't new_renderers_back_to_normal_ be always equal to
> > new_renderers_until_throttled_?
> 
> We have different parameters for # of renderers to switch states as we
discussed
> before[1]. Basically back_to_normal > until_throttled
> 
> [1] https://codereview.chromium.org/2374343002#msg9

Yeah, this enables a "dead band" to prevent thrashing across memory states.

https://codereview.chromium.org/2479673002/diff/20001/content/browser/memory/...
content/browser/memory/memory_coordinator_impl.cc:237: if (prev_remaining !=
next_remaining) {
On 2016/11/07 00:16:37, bashi1 wrote:
> How likely does this condition become true? |free_memory_until_critical_mb|
> isn't stable as far as I tested and I guess that |prev_remaining| and
> |next_remaining| wouldn't be the same in most cases.

Very unlikely to remain true, but this is a cheap comparison that avoids an
expensive IPC. The IPC costs many orders of magnitude more effort than the
comparison, so it doesn't have to be true very often for the trade-off to be
worth it.

https://codereview.chromium.org/2479673002/diff/20001/content/browser/memory/...
content/browser/memory/memory_coordinator_impl.cc:239:
NotifyRemainingGlobalBudgetToChildren();
On 2016/11/07 00:16:37, bashi1 wrote:
> On 2016/11/05 12:52:21, haraken wrote:
> > 
> > How frequently will this IPC be called?
> 
> My guess is that it's almost the same as how frequently UpdateState() is
called,
> which means that every 5 seconds in general.

Yup. Although we should likely still continue to compute it even during the
period of no action following a state change.

> Instead of pushing budgets, how about adding a pull IPC method which a
renderer
> can ask the browser the current budget?

Yeah, I'm ambivalent here. I think a pull API could be useful, but we'd likely
want some kind of caching + rate limiting.

https://codereview.chromium.org/2479673002/diff/20001/content/browser/memory/...
File content/browser/memory/memory_coordinator_impl.h (right):

https://codereview.chromium.org/2479673002/diff/20001/content/browser/memory/...
content/browser/memory/memory_coordinator_impl.h:72: MemoryState
CalculateNextState(int free_memory_until_critical_mb);
On 2016/11/05 12:52:21, haraken wrote:
> 
> int => int64_t ?

(Decided to move everything in the other direction.)

https://codereview.chromium.org/2479673002/diff/20001/content/child/memory/ch...
File content/child/memory/child_memory_coordinator_impl.cc (right):

https://codereview.chromium.org/2479673002/diff/20001/content/child/memory/ch...
content/child/memory/child_memory_coordinator_impl.cc:39: return
ToBaseMemoryState(g_child_memory_coordinator->GetCurrentMemoryState());
On 2016/11/05 12:52:22, haraken wrote:
> 
> g_child_memory_coordinator => GetInstance() ?

Done.

[bashi, 2016-11-07 23:17:08]

https://codereview.chromium.org/2479673002/diff/20001/base/memory/memory_coor...
File base/memory/memory_coordinator_proxy.h (right):

https://codereview.chromium.org/2479673002/diff/20001/base/memory/memory_coor...
base/memory/memory_coordinator_proxy.h:18: // is injected.
On 2016/11/07 19:33:41, chrisha (slow) wrote:
> On 2016/11/07 00:16:37, bashi1 wrote:
> > +1 :)
> 
> I'll do this in a follow-up CL, as it's going to be a bit of a mess to do
here.

Oh, I didn't mean I wanted this refactoring in this CL. Sorry for confusion. I
can do that if you don't have a bandwidth for this :)

https://codereview.chromium.org/2479673002/diff/20001/base/memory/memory_coor...
base/memory/memory_coordinator_proxy.h:38: int64_t GetRemainingGlobalBudget()
const;
On 2016/11/07 19:33:41, chrisha (slow) wrote:
> On 2016/11/07 00:16:37, bashi1 wrote:
> > nit: we use int for MemoryMonitor::GetFreeMemoryUntilCriticalMB(). Should we
> use
> > int64_t everywhere?
> 
> Well, an auto-scaled 'int' is sufficient for each platform, but Mojo has no
such
> thing, and forces me to use an int64_t in order to accommodate both platforms.
> I'd prefer to go the other way and keep everything an int, except for the IPC
> message itself?

I see.

https://codereview.chromium.org/2479673002/diff/20001/content/browser/memory/...
File content/browser/memory/memory_coordinator_impl.cc (right):

https://codereview.chromium.org/2479673002/diff/20001/content/browser/memory/...
content/browser/memory/memory_coordinator_impl.cc:237: if (prev_remaining !=
next_remaining) {
On 2016/11/07 19:33:41, chrisha (slow) wrote:
> On 2016/11/07 00:16:37, bashi1 wrote:
> > How likely does this condition become true? |free_memory_until_critical_mb|
> > isn't stable as far as I tested and I guess that |prev_remaining| and
> > |next_remaining| wouldn't be the same in most cases.
> 
> Very unlikely to remain true, but this is a cheap comparison that avoids an
> expensive IPC. The IPC costs many orders of magnitude more effort than the
> comparison, so it doesn't have to be true very often for the trade-off to be
> worth it.

Acknowledged.

https://codereview.chromium.org/2479673002/diff/20001/content/browser/memory/...
content/browser/memory/memory_coordinator_impl.cc:239:
NotifyRemainingGlobalBudgetToChildren();
On 2016/11/07 19:33:41, chrisha (slow) wrote:
> On 2016/11/07 00:16:37, bashi1 wrote:
> > On 2016/11/05 12:52:21, haraken wrote:
> > > 
> > > How frequently will this IPC be called?
> > 
> > My guess is that it's almost the same as how frequently UpdateState() is
> called,
> > which means that every 5 seconds in general.
> 
> Yup. Although we should likely still continue to compute it even during the
> period of no action following a state change.
> 
> > Instead of pushing budgets, how about adding a pull IPC method which a
> renderer
> > can ask the browser the current budget?
> 
> Yeah, I'm ambivalent here. I think a pull API could be useful, but we'd likely
> want some kind of caching + rate limiting.

Yeah, as haraken@ said, async API may not be useful for MCCs. I'm ambivalent
here too.

[Hannes Payer (out of office), 2016-11-11 12:55:19]

hpayer@chromium.org changed reviewers:
+ hpayer@chromium.org

[Hannes Payer (out of office), 2016-11-11 12:55:19]

https://codereview.chromium.org/2479673002/diff/20001/content/browser/memory/...
File content/browser/memory/memory_coordinator_impl.cc (right):

https://codereview.chromium.org/2479673002/diff/20001/content/browser/memory/...
content/browser/memory/memory_coordinator_impl.cc:86: expected_renderer_size_ =
kDefaultExpectedRendererSizeMB;
Unrelated to this CL: How did you come up with kDefaultExpectedRendererSizeMB?

https://codereview.chromium.org/2479673002/diff/20001/content/browser/memory/...
content/browser/memory/memory_coordinator_impl.cc:199: int renderer_count =
new_renderers_until_throttled_;
I do not understand why you use kDefaultNewRenderersUntilThrottled = 4 here for
render_count.

https://codereview.chromium.org/2479673002/diff/20001/content/browser/memory/...
content/browser/memory/memory_coordinator_impl.cc:239:
NotifyRemainingGlobalBudgetToChildren();
On 2016/11/07 23:17:08, bashi1 wrote:
> On 2016/11/07 19:33:41, chrisha (slow) wrote:
> > On 2016/11/07 00:16:37, bashi1 wrote:
> > > On 2016/11/05 12:52:21, haraken wrote:
> > > > 
> > > > How frequently will this IPC be called?
> > > 
> > > My guess is that it's almost the same as how frequently UpdateState() is
> > called,
> > > which means that every 5 seconds in general.
> > 
> > Yup. Although we should likely still continue to compute it even during the
> > period of no action following a state change.
> > 
> > > Instead of pushing budgets, how about adding a pull IPC method which a
> > renderer
> > > can ask the browser the current budget?
> > 
> > Yeah, I'm ambivalent here. I think a pull API could be useful, but we'd
likely
> > want some kind of caching + rate limiting.
> 
> Yeah, as haraken@ said, async API may not be useful for MCCs. I'm ambivalent
> here too.

Currently, I am only planning to take advantage of the memory budget in the
foreground tab. That would speak for a pull API.

[bashi, 2016-11-12 01:30:53]

https://codereview.chromium.org/2479673002/diff/20001/content/browser/memory/...
File content/browser/memory/memory_coordinator_impl.cc (right):

https://codereview.chromium.org/2479673002/diff/20001/content/browser/memory/...
content/browser/memory/memory_coordinator_impl.cc:86: expected_renderer_size_ =
kDefaultExpectedRendererSizeMB;
On 2016/11/11 12:55:19, Hannes Payer wrote:
> Unrelated to this CL: How did you come up with kDefaultExpectedRendererSizeMB?

From UMA (median of average). This is just an estimated value and we may want to
tweak when we have enough stats after MC is enabled.

https://codereview.chromium.org/2479673002/diff/20001/content/browser/memory/...
content/browser/memory/memory_coordinator_impl.cc:199: int renderer_count =
new_renderers_until_throttled_;
On 2016/11/11 12:55:19, Hannes Payer wrote:
> I do not understand why you use kDefaultNewRenderersUntilThrottled = 4 here
for
> render_count.

Because we don't know what an appropriate value for this at this point :( Our
plan is to collect stats then figure out better parameters (I'm adding UMAs for
this purpose[1]). Or we may change the heuristics we use to determine the state.
This also depends on how the current impl & parameters works.

[1]
https://docs.google.com/document/d/1Mlx7q_8thkoQtg_C7-CNA_y0SxSK0yp_ja9Zs4Hmc...

Could you elaborate your point a bit more? Do you have a better guess?

[chrisha, 2016-12-15 21:10:24]

Closing this for now. bashi@ is refactoring the base::MemoryCoordinator, and
we'll do this on top of his changes.

https://codereview.chromium.org/2479673002/diff/20001/content/browser/memory/...
File content/browser/memory/memory_coordinator_impl.cc (right):

https://codereview.chromium.org/2479673002/diff/20001/content/browser/memory/...
content/browser/memory/memory_coordinator_impl.cc:237: if (prev_remaining !=
next_remaining) {
On 2016/11/07 19:33:41, chrisha (slow) wrote:
> On 2016/11/07 00:16:37, bashi1 wrote:
> > How likely does this condition become true? |free_memory_until_critical_mb|
> > isn't stable as far as I tested and I guess that |prev_remaining| and
> > |next_remaining| wouldn't be the same in most cases.
> 
> Very unlikely to remain true, but this is a cheap comparison that avoids an
> expensive IPC. The IPC costs many orders of magnitude more effort than the
> comparison, so it doesn't have to be true very often for the trade-off to be
> worth it.

(This is a moot point with a "pull" model.)

https://codereview.chromium.org/2479673002/diff/20001/content/browser/memory/...
content/browser/memory/memory_coordinator_impl.cc:239:
NotifyRemainingGlobalBudgetToChildren();
On 2016/11/07 19:33:41, chrisha (slow) wrote:
> On 2016/11/07 00:16:37, bashi1 wrote:
> > On 2016/11/05 12:52:21, haraken wrote:
> > > 
> > > How frequently will this IPC be called?
> > 
> > My guess is that it's almost the same as how frequently UpdateState() is
> called,
> > which means that every 5 seconds in general.
> 
> Yup. Although we should likely still continue to compute it even during the
> period of no action following a state change.
> 
> > Instead of pushing budgets, how about adding a pull IPC method which a
> renderer
> > can ask the browser the current budget?
> 
> Yeah, I'm ambivalent here. I think a pull API could be useful, but we'd likely
> want some kind of caching + rate limiting.

Moved to a "pull" model.

[chrisha, 2016-12-15 21:10:34]

Description was changed from

==========
Expose MemoryCoordinator's global budget information.

This information will be used by V8 to tweak its memory usage in real time.

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

to

==========
Expose MemoryCoordinator's global budget information.

This information will be used by V8 to tweak its memory usage in real time.

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