Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2360433002:
Memory-monitor for Mac. (Closed)
DescriptionMemory-monitor for Mac.
Design Doc:
https://docs.google.com/a/chromium.org/document/d/18E5pfepxFeRjPTOZkgKP8MCmingIzHwbI0Ylq-K8McI/edit?usp=sharing
BUG=644397
Patch Set 1 #
Total comments: 1
Patch Set 2 : calculate adjustment based on poll interval; limit calculations to avoid overflow #
Total comments: 13
Patch Set 3 : rebased #Patch Set 4 : fixed typo #Patch Set 5 : rebased #
Total comments: 6
Patch Set 6 : addressed review comments #Patch Set 7 : rebased #Messages
Total messages: 53 (26 generated)
bcwhite@chromium.org changed reviewers: + chrisha@chromium.org
Not sure if I really like this algorithm or not. Let me know what you think.
chrisha@chromium.org changed reviewers: + shrike@chromium.org
Maybe shrike can comment on this, as he played a lot with the memory pressure monitor on Mac. For context: we're trying to build a simple mechanism that tells us how much memory is free until critical (paging). We can't get this explicitly on Mac, but we suspect we can learn it by tracking the amount of free memory available when a critical memory notification occurs. Thoughts?
On 2016/09/26 17:48:11, chrisha (slow) wrote: > Maybe shrike can comment on this, as he played a lot with the memory pressure > monitor on Mac. > > For context: we're trying to build a simple mechanism that tells us how much > memory is free until critical (paging). We can't get this explicitly on Mac, but > we suspect we can learn it by tracking the amount of free memory available when > a critical memory notification occurs. Thoughts? Shrike, any comments? I'm told that this is likely called every 5 seconds so I think I need to add a timer check so that the ramp-up and ramp-down occur only once every 30 seconds or so. Or if there is a constant somewhere that defines the interval, I can just divide the adjustment amount by that 30/number.
On 2016/09/29 19:21:04, bcwhite wrote: > Shrike, any comments? > > I'm told that this is likely called every 5 seconds so I think I need to add a > timer check so that the ramp-up and ramp-down occur only once every 30 seconds > or so. Or if there is a constant somewhere that defines the interval, I can > just divide the adjustment amount by that 30/number. Thank you for the ping, and my apologies for needing to ping me. Overall this is an interesting idea. Is it true that GetFreeMemoryUntilCriticalMB() will be called periodically? According to the code it maintains critical_free_mb_ which is related to how much memory must be free to stay out of the CRITICAL memory pressure state. This value is adjusted up or down during polling - up if there's moderate memory pressure, and down if there's no memory pressure. The method returns free - critical_free_mb_. With polling and the WARNING memory pressure state it seems like critical_free_mb_ can grow unbounded? I'm guessing the assumption is that that's OK because it would take very long time to overflow the int. However, let's say I launch Chrome and my machine is already in the WARNING memory pressure state and stays there for the next four hours. Let's further say that free memory at start up is 1Gb, so critical_free_mb_ starts off at 500Mb. At the end of the four hours I quit some other app that was hogging up memory, and memory pressure drops back to NORMAL. GetFreeMemoryUntilCriticalMB() will start ramping critical_free_mb_ down but by definition it will take 4 hours before it returns to 500Mb, and another 4 hours (at -2Mb per minute) to ramp down to zero. All of those eight hours Chrome is acting as if memory is constrained when in fact there's plenty. Maybe under NORMAL pressure critical_free_mb_ gets set to 0? I guess there's the fear of oscillating between NORMAL and WARNING?
https://codereview.chromium.org/2360433002/diff/1/content/browser/memory/memo... File content/browser/memory/memory_monitor_mac.cc (right): https://codereview.chromium.org/2360433002/diff/1/content/browser/memory/memo... content/browser/memory/memory_monitor_mac.cc:48: // of memory that con be used. "con" should be "can"
shrike: For context, this is to be used in the memory coordinator, which is a *stateful* replacement for memory pressure. There is a single threshold (critical), which is similarly defined on all platforms, and the memory monitory is responsible for estimating how far away we are from that limit in terms of free memory (or a negative value, to estimate how much needs to be freed in order to get out of it). The polling interval is to be the same as the existing memory pressure monitor mechanism. On Thu, 29 Sep 2016 at 18:48 <shrike@chromium.org> wrote: > > > https://codereview.chromium.org/2360433002/diff/1/content/browser/memory/memo... > File content/browser/memory/memory_monitor_mac.cc (right): > > > https://codereview.chromium.org/2360433002/diff/1/content/browser/memory/memo... > content/browser/memory/memory_monitor_mac.cc:48: // of memory that con > be used. > "con" should be "can" > > https://codereview.chromium.org/2360433002/ > -- You received this message because you are subscribed to the Google Groups "Chromium-reviews" group. To unsubscribe from this group and stop receiving emails from it, send an email to chromium-reviews+unsubscribe@chromium.org.
On 2016/09/30 03:00:20, chrisha (slow) wrote: > shrike: For context, this is to be used in the memory coordinator, which is > a *stateful* replacement for memory pressure. There is a single threshold > (critical), which is similarly defined on all platforms, and the memory > monitory is responsible for estimating how far away we are from that limit > in terms of free memory (or a negative value, to estimate how much needs to > be freed in order to get out of it). The polling interval is to be the same > as the existing memory pressure monitor mechanism. Yes, I pieced together most of that from reading GetFreeMemoryUntilCriticalMB(). (Also there is no polling interval in the Mac memory pressure monitor - that's ChromeOS only I believe.)
> With polling and the WARNING memory pressure state it seems like > critical_free_mb_ can grow unbounded? I'm guessing the assumption is that that's > OK because it would take very long time to overflow the int. However, let's say > I launch Chrome and my machine is already in the WARNING memory pressure state > and stays there for the next four hours. Let's further say that free memory at > start up is 1Gb, so critical_free_mb_ starts off at 500Mb. At the end of the > four hours I quit some other app that was hogging up memory, and memory > pressure drops back to NORMAL. GetFreeMemoryUntilCriticalMB() will start ramping > critical_free_mb_ down but by definition it will take 4 hours before it returns > to 500Mb, and another 4 hours (at -2Mb per minute) to ramp down to zero. All of > those eight hours Chrome is acting as if memory is constrained when in fact > there's plenty. Maybe under NORMAL pressure critical_free_mb_ gets set to 0? I > guess there's the fear of oscillating between NORMAL and WARNING? Good point. How about a linear increase in the threshold while WARNING (after the initial jump to 1/2 available) and an exponential decrease while NORMAL? A decrease of 20% every minute would mean it takes the system 10 minutes to be reduced by 90%. (10 rt 0.1 = 0.8) I don't think oscillating between NORMAL and WARNING is a problem. In fact, I think it's good because it would mean we're holding at a boundary where Chrome is most efficient without causing impact to the rest of the system.
On 2016/09/30 13:51:03, bcwhite wrote: > Good point. How about a linear increase in the threshold while WARNING (after > the initial jump to 1/2 available) and an exponential decrease while NORMAL? > > A decrease of 20% every minute would mean it takes the system 10 minutes to be > reduced by 90%. (10 rt 0.1 = 0.8) An exponential decrease sounds like it would be the fix. Another thought / question: are there any metrics you can collect so that we can make sure this system is performing as intended out in the wild? I just worry about some unexpected edge case like this one that hurts Chrome's performance, or causes Chrome to use memory when it should not, and us not detecting it. > I don't think oscillating between NORMAL and WARNING is a problem. In fact, I > think it's good because it would mean we're holding at a boundary where Chrome > is most efficient without causing impact to the rest of the system. OK.
Ping?
On 2016/10/10 23:45:02, chrisha (slow) wrote: > Ping? I'm waiting for the main CL to go in so I can access the constants that indicate how frequently this is called.
Ah, okay. Thanks for the reminder (you had told me that, but it slipped my vice-like mind.) On Tue, 11 Oct 2016 at 11:43 <bcwhite@chromium.org> wrote: > On 2016/10/10 23:45:02, chrisha (slow) wrote: > > Ping? > > I'm waiting for the main CL to go in so I can access the constants that > indicate > how frequently this is called. > > https://codereview.chromium.org/2360433002/ > -- You received this message because you are subscribed to the Google Groups "Chromium-reviews" group. To unsubscribe from this group and stop receiving emails from it, send an email to chromium-reviews+unsubscribe@chromium.org.
> With polling and the WARNING memory pressure state it seems like > critical_free_mb_ can grow unbounded? I'm guessing the assumption is that that's > OK because it would take very long time to overflow the int. However, let's say > I launch Chrome and my machine is already in the WARNING memory pressure state > and stays there for the next four hours. Let's further say that free memory at > start up is 1Gb, so critical_free_mb_ starts off at 500Mb. At the end of the > four hours I quit some other app that was hogging up memory, and memory > pressure drops back to NORMAL. GetFreeMemoryUntilCriticalMB() will start ramping > critical_free_mb_ down but by definition it will take 4 hours before it returns > to 500Mb, and another 4 hours (at -2Mb per minute) to ramp down to zero. All of > those eight hours Chrome is acting as if memory is constrained when in fact > there's plenty. Maybe under NORMAL pressure critical_free_mb_ gets set to 0? I > guess there's the fear of oscillating between NORMAL and WARNING? After updating the CL to do the exponential decrease and starting to update the unittest... I was suddenly struck that perhaps the better choice is simply to limit the critical_free_mb_ to the current amount of free memory. Is the WARNING state, Chrome will soon increment critical_free_mb_ to the actual amount of free memory and then hold it there. At that point, Chrome will essentially be in the CRITICAL state and reduce its memory usage as much as possible. Eventually when that other process ends, there is far more free memory than critical_free_mb_ and so Chrome can again use memory and that variable reduces further. This way, critical_free_mb_ never grows larger than where WARNING is expected so even after sudden changes, Chrome will try to avoid that state, eventually forgetting that limit altogether.
The CQ bit was checked by bcwhite@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
On 2016/11/01 12:59:27, bcwhite wrote: > > With polling and the WARNING memory pressure state it seems like > > critical_free_mb_ can grow unbounded? I'm guessing the assumption is that > that's > > OK because it would take very long time to overflow the int. However, let's > say > > I launch Chrome and my machine is already in the WARNING memory pressure > state > > and stays there for the next four hours. Let's further say that free memory at > > start up is 1Gb, so critical_free_mb_ starts off at 500Mb. At the end of the > > four hours I quit some other app that was hogging up memory, and memory > > pressure drops back to NORMAL. GetFreeMemoryUntilCriticalMB() will start > ramping > > critical_free_mb_ down but by definition it will take 4 hours before it > returns > > to 500Mb, and another 4 hours (at -2Mb per minute) to ramp down to zero. All > of > > those eight hours Chrome is acting as if memory is constrained when in fact > > there's plenty. Maybe under NORMAL pressure critical_free_mb_ gets set to 0? I > > guess there's the fear of oscillating between NORMAL and WARNING? > > After updating the CL to do the exponential decrease and starting to update the > unittest... I was suddenly struck that perhaps the better choice is simply to > limit the critical_free_mb_ to the current amount of free memory. > > Is the WARNING state, Chrome will soon increment critical_free_mb_ to the actual > amount of free memory and then hold it there. At that point, Chrome will > essentially be in the CRITICAL state and reduce its memory usage as much as > possible. Eventually when that other process ends, there is far more free > memory than critical_free_mb_ and so Chrome can again use memory and that > variable reduces further. > > This way, critical_free_mb_ never grows larger than where WARNING is expected so > even after sudden changes, Chrome will try to avoid that state, eventually > forgetting that limit altogether. Done. PTAL
Ping?
I'd prefer shrike@ comment on the logic here, if he has any more insight into memory management on Mac. https://codereview.chromium.org/2360433002/diff/20001/content/browser/memory/... File content/browser/memory/memory_monitor.h (right): https://codereview.chromium.org/2360433002/diff/20001/content/browser/memory/... content/browser/memory/memory_monitor.h:56: virtual bool GetSystemMemoryPressure(int* mem_pressure); Can't we push this to the MemoryMonitorMacDelegate specialization? https://codereview.chromium.org/2360433002/diff/20001/content/browser/memory/... File content/browser/memory/memory_monitor_mac.cc (right): https://codereview.chromium.org/2360433002/diff/20001/content/browser/memory/... content/browser/memory/memory_monitor_mac.cc:65: critical_free_mb_ = half_free; Do we have a reason to believe that 50% is the right lower bound here? https://codereview.chromium.org/2360433002/diff/20001/content/browser/memory/... content/browser/memory/memory_monitor_mac.cc:71: critical_free_mb_ = free_mem; Shouldn't this be *zero*? Critical is defined as swapping has occurred, so by definition there's no free memory left before swapping? https://codereview.chromium.org/2360433002/diff/20001/content/browser/memory/... File content/browser/memory/memory_monitor_mac.h (right): https://codereview.chromium.org/2360433002/diff/20001/content/browser/memory/... content/browser/memory/memory_monitor_mac.h:41: // An delegate with extra methods required to support Mac. A* delegate
The CQ bit was checked by bcwhite@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
https://codereview.chromium.org/2360433002/diff/20001/content/browser/memory/... File content/browser/memory/memory_monitor.h (right): https://codereview.chromium.org/2360433002/diff/20001/content/browser/memory/... content/browser/memory/memory_monitor.h:56: virtual bool GetSystemMemoryPressure(int* mem_pressure); On 2016/11/09 21:21:11, chrisha (slow) wrote: > Can't we push this to the MemoryMonitorMacDelegate specialization? I tried that but it makes tests problematic because they use classes derived from TestMemoryMonitorDelegate which derives from MemoryMonitorDelegate. So in order to both override this method and have access to the common test code, this interface has to define both. https://codereview.chromium.org/2360433002/diff/20001/content/browser/memory/... File content/browser/memory/memory_monitor_mac.cc (right): https://codereview.chromium.org/2360433002/diff/20001/content/browser/memory/... content/browser/memory/memory_monitor_mac.cc:65: critical_free_mb_ = half_free; On 2016/11/09 21:21:11, chrisha (slow) wrote: > Do we have a reason to believe that 50% is the right lower bound here? None at all. Just seemed a reasonable starting guess that would nudge Chrome into tightening its belt. I figured that merely incrementing from zero would mean that the memory pressure went largely unaddressed for numerous minutes. https://codereview.chromium.org/2360433002/diff/20001/content/browser/memory/... content/browser/memory/memory_monitor_mac.cc:71: critical_free_mb_ = free_mem; On 2016/11/09 21:21:11, chrisha (slow) wrote: > Shouldn't this be *zero*? Critical is defined as swapping has occurred, so by > definition there's no free memory left before swapping? Critical is defined as the amount of memory that can be allocated before the system begins to swap. Since CRITICAL essentially means that the system is about to swap then the amount of free memory before swap must be the current amount of free memory. Line 76 does the subtraction that will cause a zero return value. https://codereview.chromium.org/2360433002/diff/20001/content/browser/memory/... File content/browser/memory/memory_monitor_mac.h (right): https://codereview.chromium.org/2360433002/diff/20001/content/browser/memory/... content/browser/memory/memory_monitor_mac.h:41: // An delegate with extra methods required to support Mac. On 2016/11/09 21:21:11, chrisha (slow) wrote: > A* delegate Done.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by bcwhite@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was checked by bcwhite@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
https://codereview.chromium.org/2360433002/diff/20001/content/browser/memory/... File content/browser/memory/memory_monitor.h (right): https://codereview.chromium.org/2360433002/diff/20001/content/browser/memory/... content/browser/memory/memory_monitor.h:55: // or false otherwise. Expand the comment that this is only defined on some platforms, and that the meaning of the value is very much OS specific? https://codereview.chromium.org/2360433002/diff/20001/content/browser/memory/... File content/browser/memory/memory_monitor_mac.cc (right): https://codereview.chromium.org/2360433002/diff/20001/content/browser/memory/... content/browser/memory/memory_monitor_mac.cc:65: critical_free_mb_ = half_free; On 2016/11/09 22:46:54, bcwhite wrote: > On 2016/11/09 21:21:11, chrisha (slow) wrote: > > Do we have a reason to believe that 50% is the right lower bound here? > > None at all. Just seemed a reasonable starting guess that would nudge Chrome > into tightening its belt. I figured that merely incrementing from zero would > mean that the memory pressure went largely unaddressed for numerous minutes. Do you have an idea for how we could monitor the performance of this algorithm once enabled in the field? Track its responsiveness? How often its right vs wrong, etc? https://codereview.chromium.org/2360433002/diff/20001/content/browser/memory/... content/browser/memory/memory_monitor_mac.cc:71: critical_free_mb_ = free_mem; On 2016/11/09 22:46:54, bcwhite wrote: > On 2016/11/09 21:21:11, chrisha (slow) wrote: > > Shouldn't this be *zero*? Critical is defined as swapping has occurred, so by > > definition there's no free memory left before swapping? > > Critical is defined as the amount of memory that can be allocated before the > system begins to swap. Since CRITICAL essentially means that the system is > about to swap then the amount of free memory before swap must be the current > amount of free memory. > > Line 76 does the subtraction that will cause a zero return value. Ah, okay, makes sense. https://codereview.chromium.org/2360433002/diff/100001/content/browser/memory... File content/browser/memory/memory_monitor_mac.h (right): https://codereview.chromium.org/2360433002/diff/100001/content/browser/memory... content/browser/memory/memory_monitor_mac.h:46: // Returns system memory-pressure information. Define what the pressure levels are, and their meaning, on this platform?
https://codereview.chromium.org/2360433002/diff/100001/content/browser/memory... File content/browser/memory/memory_monitor_mac.cc (right): https://codereview.chromium.org/2360433002/diff/100001/content/browser/memory... content/browser/memory/memory_monitor_mac.cc:58: std::max(0LL, critical_free_mb_ - poll_interval_adjustment_mb_); This seems to have to same backoff problem I described before. If memory pressure ceases, Chrome's self-imposed memory constraint diminishes slowly. When you jump from NORMAL to WARN, critical_free_mb_ can jump from 0 to half of free memory, but when you jump from WARN to NORMAL, critical_free_mb_ drops at just poll_interval_adjustment_mb_. https://codereview.chromium.org/2360433002/diff/100001/content/browser/memory... content/browser/memory/memory_monitor_mac.cc:67: critical_free_mb_ += poll_interval_adjustment_mb_; I think the idea of adjusting memory use so that you avoid triggering memory pressure is interesting, but I'm not sure it's achievable without imposing potentially unreasonable or unneeded memory constraints on Chrome (at least on the Mac). I'm still trying to put my finger on the exact issue. I think part of it is that this synthetic signal seems to behave as if Chrome is the only application driving memory pressure - putting further and further constraints on Chrome's memory use while in the WARN state, for example, does not in any way guarantee any effect on memory pressure. Conversely, a memory-intensive application (or several applications running at the same time) would push Chrome to self-imposed memory constraints. Another problem is I don't think WARN pressure on the Mac is a consistent/good indicator that an app needs to change its behavior. On the Mac, WARN means that free RAM is exhausted and the OS is now compressing memory pages to make room. If the OS is compressing not-recently-used pages from another app to make room for Chrome, that is not necessarily a bad thing. In the old days those pages would have been swapped out, but with GPU-assisted compression the performance hit is much smaller. And it seems to me that a system could stay in the WARN state for an extended period of time (triggered by the compression of just a single page of RAM). During this time Chrome would force itself to run in a memory-constrained state when in fact there might be plenty of RAM that can be reclaimed by the OS. You are also basing your calculations on the total free memory from the VM system, which may not a good measure of what's actually available on the system. To quote some research I did on the meaning of macOS memory stats in https://docs.google.com/a/google.com/document/d/1ydmF9DMuEVPUiKkMnu2Bnb0RwCah...: ...when a process removes virtual memory objects, there is no requirement that the OS immediately unassign the RAM that backed those objects. The OS may decide it’s better to keep those pages assigned to the process (it can easily reassign them if needed by other processes). As loads and stores cause the OS to assign RAM to a process, the OS may decide to assign even more RAM than necessary, to lower the overhead of future references. In these cases, RSIZE will report a larger physical memory allocation than is actually accounted for by RPRVT + RSHRD. So, it might be the case that the OS has given a bunch of memory to a process in anticipation of its memory needs, but those needs might not pan out in which case it will just reclaim that memory when another app needs more RAM. I'm not 100% sure how this kind of overallocation affects the free memory value returned by GetSystemMemoryInfo() (it hurts my head a little keeping the VM vs. physical RAM allocations straight). The design of this system really needs to be laid out in a design doc, where potential issues can be easily discussed and the overall design easily evaluated.
The CQ bit was checked by bcwhite@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)
https://codereview.chromium.org/2360433002/diff/20001/content/browser/memory/... File content/browser/memory/memory_monitor.h (right): https://codereview.chromium.org/2360433002/diff/20001/content/browser/memory/... content/browser/memory/memory_monitor.h:55: // or false otherwise. On 2016/11/14 18:14:31, chrisha (slow) wrote: > Expand the comment that this is only defined on some platforms, and that the > meaning of the value is very much OS specific? Done. https://codereview.chromium.org/2360433002/diff/20001/content/browser/memory/... File content/browser/memory/memory_monitor_mac.cc (right): https://codereview.chromium.org/2360433002/diff/20001/content/browser/memory/... content/browser/memory/memory_monitor_mac.cc:65: critical_free_mb_ = half_free; On 2016/11/14 18:14:31, chrisha (slow) wrote: > On 2016/11/09 22:46:54, bcwhite wrote: > > On 2016/11/09 21:21:11, chrisha (slow) wrote: > > > Do we have a reason to believe that 50% is the right lower bound here? > > > > None at all. Just seemed a reasonable starting guess that would nudge Chrome > > into tightening its belt. I figured that merely incrementing from zero would > > mean that the memory pressure went largely unaddressed for numerous minutes. > > Do you have an idea for how we could monitor the performance of this algorithm > once enabled in the field? Track its responsiveness? How often its right vs > wrong, etc? I could create a metric that counts the number of consecutive pressure warnings. If it's small, then this is sufficient but if it's large then it's not enough. The reliability of this is questionable, though, because it'll likely be more dependent on what the "greater chrome" does and what else is happening in the system. I could similarly count the number of such warnings before a CRITICAL event which would indicate that it wasn't enough... but again I think there would be more "noise" than "signal". https://codereview.chromium.org/2360433002/diff/100001/content/browser/memory... File content/browser/memory/memory_monitor_mac.cc (right): https://codereview.chromium.org/2360433002/diff/100001/content/browser/memory... content/browser/memory/memory_monitor_mac.cc:58: std::max(0LL, critical_free_mb_ - poll_interval_adjustment_mb_); On 2016/11/14 22:12:29, shrike wrote: > This seems to have to same backoff problem I described before. If memory > pressure ceases, Chrome's self-imposed memory constraint diminishes slowly. > When you jump from NORMAL to WARN, critical_free_mb_ can jump from 0 to half of > free memory, but when you jump from WARN to NORMAL, critical_free_mb_ drops at > just poll_interval_adjustment_mb_. Not quite because the maximum value of critical_free_mb_ is now the amount of free memory during WARN/CRITICAL. That can never be very large -- if it was, then there wouldn't be memory pressure. If more frees up and the pressure goes away then it's a reasonable assumption that pressure would resume at the same point which means that the existing value is actually accurate. But because things change on the system over time, it's probably best to let that reduce so we don't get stuck in some wrong understanding. https://codereview.chromium.org/2360433002/diff/100001/content/browser/memory... content/browser/memory/memory_monitor_mac.cc:67: critical_free_mb_ += poll_interval_adjustment_mb_; On 2016/11/14 22:12:29, shrike wrote: > I think the idea of adjusting memory use so that you avoid triggering memory > pressure is interesting, but I'm not sure it's achievable without imposing > potentially unreasonable or unneeded memory constraints on Chrome (at least on > the Mac). I'm still trying to put my finger on the exact issue. I think part of > it is that this synthetic signal seems to behave as if Chrome is the only > application driving memory pressure - putting further and further constraints on > Chrome's memory use while in the WARN state, for example, does not in any way > guarantee any effect on memory pressure. Conversely, a memory-intensive > application (or several applications running at the same time) would push Chrome > to self-imposed memory constraints. > > Another problem is I don't think WARN pressure on the Mac is a consistent/good > indicator that an app needs to change its behavior. On the Mac, WARN means that > free RAM is exhausted and the OS is now compressing memory pages to make room. > If the OS is compressing not-recently-used pages from another app to make room > for Chrome, that is not necessarily a bad thing. In the old days those pages > would have been swapped out, but with GPU-assisted compression the performance > hit is much smaller. And it seems to me that a system could stay in the WARN > state for an extended period of time (triggered by the compression of just a > single page of RAM). During this time Chrome would force itself to run in a > memory-constrained state when in fact there might be plenty of RAM that can be > reclaimed by the OS. > > You are also basing your calculations on the total free memory from the VM > system, which may not a good measure of what's actually available on the system. > To quote some research I did on the meaning of macOS memory stats in > https://docs.google.com/a/google.com/document/d/1ydmF9DMuEVPUiKkMnu2Bnb0RwCah...: > > ...when a process removes virtual memory objects, there is no requirement that > the OS immediately unassign the RAM that backed those objects. The OS may decide > it’s better to keep those pages assigned to the process (it can easily reassign > them if needed by other processes). As loads and stores cause the OS to assign > RAM to a process, the OS may decide to assign even more RAM than necessary, to > lower the overhead of future references. In these cases, RSIZE will report a > larger physical memory allocation than is actually accounted for by RPRVT + > RSHRD. > > So, it might be the case that the OS has given a bunch of memory to a process in > anticipation of its memory needs, but those needs might not pan out in which > case it will just reclaim that memory when another app needs more RAM. I'm not > 100% sure how this kind of overallocation affects the free memory value returned > by GetSystemMemoryInfo() (it hurts my head a little keeping the VM vs. physical > RAM allocations straight). > > The design of this system really needs to be laid out in a design doc, where > potential issues can be easily discussed and the overall design easily > evaluated. It's certain that nothing is going to be perfect and, I suspect, even the best plans are going to be only a 1st order approximation of "good". I'm open to better algorithms but this is a feedback control system and after years of work on control systems, I can confidently say that the simpler, the better. Regarding the meanings of the three definitions: I was unable to find any official definition of what they *mean* to the program receiving it other that what one can garner from the words. It may be calculated as you say but that could change so trying to plan based on it seems be a dangerous choice. https://codereview.chromium.org/2360433002/diff/100001/content/browser/memory... File content/browser/memory/memory_monitor_mac.h (right): https://codereview.chromium.org/2360433002/diff/100001/content/browser/memory... content/browser/memory/memory_monitor_mac.h:46: // Returns system memory-pressure information. On 2016/11/14 18:14:31, chrisha (slow) wrote: > Define what the pressure levels are, and their meaning, on this platform? Done.
On 2016/11/15 23:15:09, bcwhite wrote: > On 2016/11/14 22:12:29, shrike wrote: > > This seems to have to same backoff problem I described before. If memory > > pressure ceases, Chrome's self-imposed memory constraint diminishes slowly. > > When you jump from NORMAL to WARN, critical_free_mb_ can jump from 0 to half > of > > free memory, but when you jump from WARN to NORMAL, critical_free_mb_ drops at > > just poll_interval_adjustment_mb_. > > Not quite because the maximum value of critical_free_mb_ is now the amount of > free memory during WARN/CRITICAL. That can never be very large -- if it was, > then there wouldn't be memory pressure. If free memory is 100MB and the MB change rate is 1Mb per second, critical_free_mb_ jumps from 0 to 50Mb and then takes 50s to climb to 100Mb, but it will take 100s for it to drop down to 0. So recovery time is still not balanced. > https://codereview.chromium.org/2360433002/diff/100001/content/browser/memory... > content/browser/memory/memory_monitor_mac.cc:67: critical_free_mb_ += > poll_interval_adjustment_mb_; > On 2016/11/14 22:12:29, shrike wrote: > > I think the idea of adjusting memory use so that you avoid triggering memory > > pressure is interesting, but I'm not sure it's achievable without imposing > > potentially unreasonable or unneeded memory constraints on Chrome (at least on > > the Mac). I'm still trying to put my finger on the exact issue. I think part > of > > it is that this synthetic signal seems to behave as if Chrome is the only > > application driving memory pressure - putting further and further constraints > on > > Chrome's memory use while in the WARN state, for example, does not in any way > > guarantee any effect on memory pressure. Conversely, a memory-intensive > > application (or several applications running at the same time) would push > Chrome > > to self-imposed memory constraints. > > > > Another problem is I don't think WARN pressure on the Mac is a consistent/good > > indicator that an app needs to change its behavior. On the Mac, WARN means > that > > free RAM is exhausted and the OS is now compressing memory pages to make room. > > If the OS is compressing not-recently-used pages from another app to make room > > for Chrome, that is not necessarily a bad thing. In the old days those pages > > would have been swapped out, but with GPU-assisted compression the performance > > hit is much smaller. And it seems to me that a system could stay in the WARN > > state for an extended period of time (triggered by the compression of just a > > single page of RAM). During this time Chrome would force itself to run in a > > memory-constrained state when in fact there might be plenty of RAM that can be > > reclaimed by the OS. > > > > You are also basing your calculations on the total free memory from the VM > > system, which may not a good measure of what's actually available on the > system. > > To quote some research I did on the meaning of macOS memory stats in > > > https://docs.google.com/a/google.com/document/d/1ydmF9DMuEVPUiKkMnu2Bnb0RwCah...: > > > > ...when a process removes virtual memory objects, there is no requirement that > > the OS immediately unassign the RAM that backed those objects. The OS may > decide > > it’s better to keep those pages assigned to the process (it can easily > reassign > > them if needed by other processes). As loads and stores cause the OS to assign > > RAM to a process, the OS may decide to assign even more RAM than necessary, to > > lower the overhead of future references. In these cases, RSIZE will report a > > larger physical memory allocation than is actually accounted for by RPRVT + > > RSHRD. > > > > So, it might be the case that the OS has given a bunch of memory to a process > in > > anticipation of its memory needs, but those needs might not pan out in which > > case it will just reclaim that memory when another app needs more RAM. I'm not > > 100% sure how this kind of overallocation affects the free memory value > returned > > by GetSystemMemoryInfo() (it hurts my head a little keeping the VM vs. > physical > > RAM allocations straight). > > > > The design of this system really needs to be laid out in a design doc, where > > potential issues can be easily discussed and the overall design easily > > evaluated. > > It's certain that nothing is going to be perfect and, I suspect, even the best > plans are going to be only a 1st order approximation of "good". I'm open to > better algorithms but this is a feedback control system and after years of work > on control systems, I can confidently say that the simpler, the better. I have had pretty much no experience with control systems, but I can see simpler being better. As for possible better algorithms, they need to be discussed in a design doc. This synthetic memory pressure system will govern available memory to Chrome, which will directly affect its performance. Getting this system wrong, due to an unintended consequence or other design flaw, means impacting Chrome's performance on the Mac, possibly severely. We don't want to land this code without being confident that it does what we intend it to do (or without the proper safeguards in place to catch it if it does not).
> > Not quite because the maximum value of critical_free_mb_ is now the amount of > > free memory during WARN/CRITICAL. That can never be very large -- if it was, > > then there wouldn't be memory pressure. > > If free memory is 100MB and the MB change rate is 1Mb per second, > critical_free_mb_ jumps from 0 to 50Mb and then takes 50s to climb to 100Mb, but > it will take 100s for it to drop down to 0. So recovery time is still not > balanced. No, it's not balanced but previously it grew without bound which was a problem. However, a desire for "balance" assumes that we want it to return to zero. Realistically, we only want it to return to the point where memory pressure events would start to occur again which, logically, would be max(all free memory values when CRITICAL occurs). Having it continue down to zero is just because I don't feel we can trust using memory pressure events reveal accurate minimum memory levels.
On 2016/11/15 23:53:00, bcwhite wrote: > No, it's not balanced but previously it grew without bound which was a problem. > However, a desire for "balance" assumes that we want it to return to zero. > Realistically, we only want it to return to the point where memory pressure > events would start to occur again which, logically, would be max(all free memory > values when CRITICAL occurs). Having it continue down to zero is just because I > don't feel we can trust using memory pressure events reveal accurate minimum > memory levels. I guess my concern is that slowly bleeding critical_free_mb_ down to 0 prolongs the simulated memory pressure cnodition when there is in fact no actual memory pressure on the system (by definition critical_free_mb_ declining means we're in the NORMAL state). The imbalance gives the system a bias towards signaling that there's memory pressure.
On 2016/11/16 00:01:41, shrike wrote: > On 2016/11/15 23:53:00, bcwhite wrote: > > No, it's not balanced but previously it grew without bound which was a > problem. > > However, a desire for "balance" assumes that we want it to return to zero. > > Realistically, we only want it to return to the point where memory pressure > > events would start to occur again which, logically, would be max(all free > memory > > values when CRITICAL occurs). Having it continue down to zero is just because > I > > don't feel we can trust using memory pressure events reveal accurate minimum > > memory levels. > > I guess my concern is that slowly bleeding critical_free_mb_ down to 0 prolongs > the simulated memory pressure cnodition when there is in fact no actual memory > pressure on the system (by definition critical_free_mb_ declining means we're in > the NORMAL state). The imbalance gives the system a bias towards signaling that > there's memory pressure. I don't think it does. There's more memory than the threshold so it's returning a positive number meaning that more memory is available for use, just not the full "free_mem" amount of it. Under all conditions, the monitor will at a minimum say that it's safe to use all the memory up to the level where it last saw memory pressure, possibly even more due to the continual decline of the threshold. In essence, WARN will create a "soft landing" for Chrome because "available memory" reduces by 1/2 then continues to decrease linearly. CRITICAL will create a "hard landing" because Chrome will be immediately told there is zero free memory. NORMAL will always report positive free memory unless some weird inversion happens where WARN/CRITICAL occurs with X free memory but then NORMAL happens with Y free memory where Y < X. I don't see that being likely but the decrease-to-zero will eventually correct it. I suppose I should add an additional condition so that NORMAL can never return a negative amount should that inversion actually occur.
On 2016/11/16 01:30:06, bcwhite wrote: > On 2016/11/16 00:01:41, shrike wrote: > > On 2016/11/15 23:53:00, bcwhite wrote: > > > No, it's not balanced but previously it grew without bound which was a > > problem. > > > However, a desire for "balance" assumes that we want it to return to zero. > > > Realistically, we only want it to return to the point where memory pressure > > > events would start to occur again which, logically, would be max(all free > > memory > > > values when CRITICAL occurs). Having it continue down to zero is just > because > > I > > > don't feel we can trust using memory pressure events reveal accurate minimum > > > memory levels. > > > > I guess my concern is that slowly bleeding critical_free_mb_ down to 0 > prolongs > > the simulated memory pressure cnodition when there is in fact no actual memory > > pressure on the system (by definition critical_free_mb_ declining means we're > in > > the NORMAL state). The imbalance gives the system a bias towards signaling > that > > there's memory pressure. > > I don't think it does. There's more memory than the threshold so it's returning > a positive number meaning that more memory is available for use, just not the > full "free_mem" amount of it. > > Under all conditions, the monitor will at a minimum say that it's safe to use > all the memory up to the level where it last saw memory pressure, possibly even > more due to the continual decline of the threshold. > > In essence, WARN will create a "soft landing" for Chrome because "available > memory" reduces by 1/2 then continues to decrease linearly. CRITICAL will > create a "hard landing" because Chrome will be immediately told there is zero > free memory. > > NORMAL will always report positive free memory unless some weird inversion > happens where WARN/CRITICAL occurs with X free memory but then NORMAL happens > with Y free memory where Y < X. I don't see that being likely but the > decrease-to-zero will eventually correct it. I suppose I should add an > additional condition so that NORMAL can never return a negative amount should > that inversion actually occur. Here you have mentioned some of the implications of and assumptions about non-zero critical_free_mb_, which are not clear from the code. We need a design doc that lays out the motivations and assumptions behind the design, and clearly describes how the proposed synthetic memory pressure system will work (at least on the Mac - I don't know if this particular design applies to other platforms).
The CQ bit was checked by bcwhite@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: linux_chromium_chromeos_compile_dbg_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...) linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...) linux_chromium_clobber_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
Patchset #5 (id:80001) has been deleted
Description was changed from ========== Memory-monitor for Mac. BUG=644397 ========== to ========== Memory-monitor for Mac. Design Doc: https://docs.google.com/a/chromium.org/document/d/18E5pfepxFeRjPTOZkgKP8MCmin... BUG=644397 ==========
On 2016/11/16 18:48:28, shrike wrote: > On 2016/11/16 01:30:06, bcwhite wrote: > > On 2016/11/16 00:01:41, shrike wrote: > > > On 2016/11/15 23:53:00, bcwhite wrote: > > > > No, it's not balanced but previously it grew without bound which was a > > > problem. > > > > However, a desire for "balance" assumes that we want it to return to zero. > > > > > Realistically, we only want it to return to the point where memory > pressure > > > > events would start to occur again which, logically, would be max(all free > > > memory > > > > values when CRITICAL occurs). Having it continue down to zero is just > > because > > > I > > > > don't feel we can trust using memory pressure events reveal accurate > minimum > > > > memory levels. > > > > > > I guess my concern is that slowly bleeding critical_free_mb_ down to 0 > > prolongs > > > the simulated memory pressure cnodition when there is in fact no actual > memory > > > pressure on the system (by definition critical_free_mb_ declining means > we're > > in > > > the NORMAL state). The imbalance gives the system a bias towards signaling > > that > > > there's memory pressure. > > > > I don't think it does. There's more memory than the threshold so it's > returning > > a positive number meaning that more memory is available for use, just not the > > full "free_mem" amount of it. > > > > Under all conditions, the monitor will at a minimum say that it's safe to use > > all the memory up to the level where it last saw memory pressure, possibly > even > > more due to the continual decline of the threshold. > > > > In essence, WARN will create a "soft landing" for Chrome because "available > > memory" reduces by 1/2 then continues to decrease linearly. CRITICAL will > > create a "hard landing" because Chrome will be immediately told there is zero > > free memory. > > > > NORMAL will always report positive free memory unless some weird inversion > > happens where WARN/CRITICAL occurs with X free memory but then NORMAL happens > > with Y free memory where Y < X. I don't see that being likely but the > > decrease-to-zero will eventually correct it. I suppose I should add an > > additional condition so that NORMAL can never return a negative amount should > > that inversion actually occur. > > Here you have mentioned some of the implications of and assumptions about > non-zero critical_free_mb_, which are not clear from the code. We need a design > doc that lays out the motivations and assumptions behind the design, and clearly > describes how the proposed synthetic memory pressure system will work (at least > on the Mac - I don't know if this particular design applies to other platforms). Quick Design Doc: https://docs.google.com/a/chromium.org/document/d/18E5pfepxFeRjPTOZkgKP8MCmin...
Closing this since it seems a new way of doing things is being created. |
