Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1105293004:
Add mode to reduce memory usage in idle notification. (Closed)
DescriptionAdd mode to reduce memory usage in idle notification.
While the mutator is active, the idle time handler optimizes for latency by doing only incremental steps and scavenges.
When the mutator becomes inactive, the idle time handler forces few incremental GCs to reclaim memory and then stops until mutator is active again.
BUG=460090
LOG=N
Committed: https://crrev.com/ae6a0b807574d4349be9c45428960ac7ecd3679a
Cr-Commit-Position: refs/heads/master@{#28300}
Patch Set 1 #Patch Set 2 : #Patch Set 3 : Add tests #Patch Set 4 : #Patch Set 5 : x #
Total comments: 15
Patch Set 6 : Address comments #
Total comments: 26
Patch Set 7 : Address comments #
Total comments: 4
Patch Set 8 : Add checks in tests #
Messages
Total messages: 18 (4 generated)
ulan@chromium.org changed reviewers: + hpayer@chromium.org
PTAL
Looking good overall. Please run the smoothness test to see if we tank latency. Please add unit tests for the helper functions like NextMode. https://codereview.chromium.org/1105293004/diff/80001/src/heap/gc-idle-time-h... File src/heap/gc-idle-time-handler.cc (right): https://codereview.chromium.org/1105293004/diff/80001/src/heap/gc-idle-time-h... src/heap/gc-idle-time-handler.cc:198: // In kReduceLatency mode the handler does only incremental steps and does not does only start incremental marking if can_... https://codereview.chromium.org/1105293004/diff/80001/src/heap/gc-idle-time-h... src/heap/gc-idle-time-handler.cc:213: // kReduceMemory => kReduceLatency transition is there were N mutator GCs happens if N mutator GCs were performed meaning the mutator is active https://codereview.chromium.org/1105293004/diff/80001/src/heap/gc-idle-time-h... src/heap/gc-idle-time-handler.cc:256: GCIdleTimeAction GCIdleTimeHandler::Step(double idle_time_in_ms, Can we find a better name here, to not confuse it with idle "steps"? What about Action? https://codereview.chromium.org/1105293004/diff/80001/src/heap/gc-idle-time-h... src/heap/gc-idle-time-handler.cc:313: ? kLongIdleNotificationsBeforeMutatorIsIdle why + kLongIdleNotificationsBeforeMutatorIsIdle? https://codereview.chromium.org/1105293004/diff/80001/src/heap/gc-idle-time-h... File src/heap/gc-idle-time-handler.h (right): https://codereview.chromium.org/1105293004/diff/80001/src/heap/gc-idle-time-h... src/heap/gc-idle-time-handler.h:53: static GCIdleTimeAction Scavenge() { scavenge could also respect the reduce_memory flag and new space size is reduced only in memory mode. also leave a todo there and do that in a separate cl. https://codereview.chromium.org/1105293004/diff/80001/src/heap/gc-idle-time-h... src/heap/gc-idle-time-handler.h:149: // The number of mutator GCs before transitioning from the kDone mode. to kReduceLatency https://codereview.chromium.org/1105293004/diff/80001/src/heap/heap.cc File src/heap/heap.cc (right): https://codereview.chromium.org/1105293004/diff/80001/src/heap/heap.cc#newcod... src/heap/heap.cc:4650: if (incremental_marking()->IsStopped()) { please add a TODO that start should take reduce_memory of DO_INCREMENTAL_MARKING into account
Thanks, I uploaded new PS. > Please add unit tests for the helper functions like NextMode. This is tested as part of the public interface (the Compute function). To test the private functions separately, we would need to expose the private state and allow setting the mode and counters outside the class. This would complicate code. If you're ok with it, I would prefer not testing private functions. wdyt? https://codereview.chromium.org/1105293004/diff/80001/src/heap/gc-idle-time-h... File src/heap/gc-idle-time-handler.cc (right): https://codereview.chromium.org/1105293004/diff/80001/src/heap/gc-idle-time-h... src/heap/gc-idle-time-handler.cc:198: // In kReduceLatency mode the handler does only incremental steps and does not On 2015/05/04 11:20:36, Hannes Payer wrote: > does only start incremental marking if can_... Done. https://codereview.chromium.org/1105293004/diff/80001/src/heap/gc-idle-time-h... src/heap/gc-idle-time-handler.cc:213: // kReduceMemory => kReduceLatency transition is there were N mutator GCs On 2015/05/04 11:20:36, Hannes Payer wrote: > happens if N mutator GCs were performed meaning the mutator is active Done. https://codereview.chromium.org/1105293004/diff/80001/src/heap/gc-idle-time-h... src/heap/gc-idle-time-handler.cc:256: GCIdleTimeAction GCIdleTimeHandler::Step(double idle_time_in_ms, On 2015/05/04 11:20:36, Hannes Payer wrote: > Can we find a better name here, to not confuse it with idle "steps"? > What about Action? Done. https://codereview.chromium.org/1105293004/diff/80001/src/heap/gc-idle-time-h... src/heap/gc-idle-time-handler.cc:313: ? kLongIdleNotificationsBeforeMutatorIsIdle On 2015/05/04 11:20:36, Hannes Payer wrote: > why + kLongIdleNotificationsBeforeMutatorIsIdle? So that the next call to idle notification immediately goes to reduce memory without waiting for kLongIdleNotificationsBeforeMutatorIsIdle notifications. https://codereview.chromium.org/1105293004/diff/80001/src/heap/gc-idle-time-h... File src/heap/gc-idle-time-handler.h (right): https://codereview.chromium.org/1105293004/diff/80001/src/heap/gc-idle-time-h... src/heap/gc-idle-time-handler.h:53: static GCIdleTimeAction Scavenge() { On 2015/05/04 11:20:37, Hannes Payer wrote: > scavenge could also respect the reduce_memory flag and new space size is reduced > only in memory mode. also leave a todo there and do that in a separate cl. Done. https://codereview.chromium.org/1105293004/diff/80001/src/heap/gc-idle-time-h... src/heap/gc-idle-time-handler.h:149: // The number of mutator GCs before transitioning from the kDone mode. On 2015/05/04 11:20:36, Hannes Payer wrote: > to kReduceLatency Done. https://codereview.chromium.org/1105293004/diff/80001/src/heap/heap.cc File src/heap/heap.cc (right): https://codereview.chromium.org/1105293004/diff/80001/src/heap/heap.cc#newcod... src/heap/heap.cc:4650: if (incremental_marking()->IsStopped()) { On 2015/05/04 11:20:37, Hannes Payer wrote: > please add a TODO that start should take reduce_memory of DO_INCREMENTAL_MARKING > into account Done.
We would have to move the counters to heap, and add them to the HeapState to make testing nice. Well, we probably don't have to go all the way. The test you wrote look fine. So, fine with me. LGTM, with just one nit. FYI Ross, this CL removes kMaxNoProgressIdleTimesPerIdleRound and goes to done after memory reduction mode. https://codereview.chromium.org/1105293004/diff/80001/src/heap/gc-idle-time-h... File src/heap/gc-idle-time-handler.cc (right): https://codereview.chromium.org/1105293004/diff/80001/src/heap/gc-idle-time-h... src/heap/gc-idle-time-handler.cc:313: ? kLongIdleNotificationsBeforeMutatorIsIdle On 2015/05/05 12:10:13, ulan wrote: > On 2015/05/04 11:20:36, Hannes Payer wrote: > > why + kLongIdleNotificationsBeforeMutatorIsIdle? > > So that the next call to idle notification immediately goes to reduce memory > without waiting for kLongIdleNotificationsBeforeMutatorIsIdle notifications. OK, makes sense for that case. https://codereview.chromium.org/1105293004/diff/100001/src/heap/gc-idle-time-... File src/heap/gc-idle-time-handler.cc (right): https://codereview.chromium.org/1105293004/diff/100001/src/heap/gc-idle-time-... src/heap/gc-idle-time-handler.cc:312: long_idle_notifications_ += (idle_time_in_ms >= kMaxLongIdleTime) This will never trigger, since idle_time_in_ms will always be smaller than kMAxLongIdleTime because of the deadline based implementation.
erik.corry@gmail.com changed reviewers: + erik.corry@gmail.com
https://codereview.chromium.org/1105293004/diff/100001/src/heap/gc-idle-time-... File src/heap/gc-idle-time-handler.cc (right): https://codereview.chromium.org/1105293004/diff/100001/src/heap/gc-idle-time-... src/heap/gc-idle-time-handler.cc:198: // In kReduceLatency mode the handler does only start incremental marking does only start -> only starts https://codereview.chromium.org/1105293004/diff/100001/src/heap/gc-idle-time-... src/heap/gc-idle-time-handler.cc:199: // if can_start_incremental_marking is false. This contradiction indicates to me that can_start_incremental_marking is misnamed. https://codereview.chromium.org/1105293004/diff/100001/src/heap/gc-idle-time-... src/heap/gc-idle-time-handler.cc:211: // kReduceMemory => kDone transition happens after X idle GCs. Probably not for this CL, but in the longer run I would prefer a different, less hard-coded rule: We need to do some minimal number, perhaps 2. After that I feel we should continue as long as we manage to free up pages of memory. That might be 0 more GCs or a large number if we are gradually compacting large numbers of fragmented pages. https://codereview.chromium.org/1105293004/diff/100001/src/heap/gc-idle-time-... src/heap/gc-idle-time-handler.cc:213: // kReduceMemory => kReduceLatency transition happens if N mutator GCs Should N be 1? If any allocation-provoked GCs are happening at all then the mutator is active and we should move to latency mode. https://codereview.chromium.org/1105293004/diff/100001/src/heap/gc-idle-time-... src/heap/gc-idle-time-handler.cc:310: ResetCounters(); A few words on the intention here? What I can see is that if scavenges and total mark-compacts add up to more than just the incremental portion of the mark-compacts we will zero all three counters, and also zero the long_idle_notifications_ counter. Why? It looks like the scavenges_ count will be zeroed on every idle notification, even a 1ms notification. https://codereview.chromium.org/1105293004/diff/100001/src/heap/gc-idle-time-... src/heap/gc-idle-time-handler.cc:314: : (idle_time_in_ms >= kMinLongIdleTime); Implicit cast of a bool to 0 or 1 is not good style. Instead you could wrap the whole += in an "if" that checks whether the idle time is < kMinLongIdleTime https://codereview.chromium.org/1105293004/diff/100001/src/heap/gc-idle-time-... File src/heap/gc-idle-time-handler.h (right): https://codereview.chromium.org/1105293004/diff/100001/src/heap/gc-idle-time-... src/heap/gc-idle-time-handler.h:152: static const int kGCsBeforeMutatorIsActive = 7; These 7 GCs will normally be scavenges, so we are talking about 7x8Mbytes of newspace allocation. Perhaps we could make this be in terms of Mbytes allocated, not GCs. That way, if we experiment with different newspace sizes we are not simultaneously making big changes to the heuristics around oldspace collection timing. https://codereview.chromium.org/1105293004/diff/100001/test/unittests/heap/gc... File test/unittests/heap/gc-idle-time-handler-unittest.cc (right): https://codereview.chromium.org/1105293004/diff/100001/test/unittests/heap/gc... test/unittests/heap/gc-idle-time-handler-unittest.cc:61: EXPECT_EQ(true, action.reduce_memory); EXPECT_TRUE(action.reduce_memory) https://codereview.chromium.org/1105293004/diff/100001/test/unittests/heap/gc... test/unittests/heap/gc-idle-time-handler-unittest.cc:71: handler()->NotifyScavenge(); In this test there are 7 GCs in a row without any idle notification, but I don't think that's realistic or what we want to test.
Thank you for reviewing! I uploaded new PS. https://codereview.chromium.org/1105293004/diff/100001/src/heap/gc-idle-time-... File src/heap/gc-idle-time-handler.cc (right): https://codereview.chromium.org/1105293004/diff/100001/src/heap/gc-idle-time-... src/heap/gc-idle-time-handler.cc:198: // In kReduceLatency mode the handler does only start incremental marking On 2015/05/05 21:38:52, Erik Corry wrote: > does only start -> only starts Done. https://codereview.chromium.org/1105293004/diff/100001/src/heap/gc-idle-time-... src/heap/gc-idle-time-handler.cc:199: // if can_start_incremental_marking is false. On 2015/05/05 21:38:52, Erik Corry wrote: > This contradiction indicates to me that can_start_incremental_marking is > misnamed. Is should_start_incremental_marking better? I can rename it in a separate CL. https://codereview.chromium.org/1105293004/diff/100001/src/heap/gc-idle-time-... src/heap/gc-idle-time-handler.cc:211: // kReduceMemory => kDone transition happens after X idle GCs. On 2015/05/05 21:38:52, Erik Corry wrote: > Probably not for this CL, but in the longer run I would prefer a different, less > hard-coded rule: > We need to do some minimal number, perhaps 2. After that I feel we should > continue as long as we manage to free up pages of memory. That might be 0 more > GCs or a large number if we are gradually compacting large numbers of fragmented > pages. I will follow up with CL that uses the 'reduce_memory' flag to do more aggressive compaction. Let's check after that CL if 3 GCs are good enough or we need smarter logic here. https://codereview.chromium.org/1105293004/diff/100001/src/heap/gc-idle-time-... src/heap/gc-idle-time-handler.cc:213: // kReduceMemory => kReduceLatency transition happens if N mutator GCs On 2015/05/05 21:38:52, Erik Corry wrote: > Should N be 1? If any allocation-provoked GCs are happening at all then the > mutator is active and we should move to latency mode. This is to prevent back-and-forth transitions from latency mode to memory mode if the mutator is almost idle: does a scavenge once in a while. https://codereview.chromium.org/1105293004/diff/100001/src/heap/gc-idle-time-... src/heap/gc-idle-time-handler.cc:310: ResetCounters(); On 2015/05/05 21:38:52, Erik Corry wrote: > A few words on the intention here? What I can see is that if scavenges and > total mark-compacts add up to more than just the incremental portion of the > mark-compacts we will zero all three counters, and also zero the > long_idle_notifications_ counter. Why? It looks like the scavenges_ count will > be zeroed on every idle notification, even a 1ms notification. I refactored this to be more explicit and added description for long_idle_notifications_ in the header: // The number of long idle notifications with no mutator GC happening // between the notifications. https://codereview.chromium.org/1105293004/diff/100001/src/heap/gc-idle-time-... src/heap/gc-idle-time-handler.cc:312: long_idle_notifications_ += (idle_time_in_ms >= kMaxLongIdleTime) On 2015/05/05 20:51:16, Hannes Payer wrote: > This will never trigger, since idle_time_in_ms will always be smaller than > kMAxLongIdleTime because of the deadline based implementation. Changed this to kLargeLongIdleTime. https://codereview.chromium.org/1105293004/diff/100001/src/heap/gc-idle-time-... src/heap/gc-idle-time-handler.cc:314: : (idle_time_in_ms >= kMinLongIdleTime); On 2015/05/05 21:38:52, Erik Corry wrote: > Implicit cast of a bool to 0 or 1 is not good style. Instead you could wrap the > whole += in an "if" that checks whether the idle time is < kMinLongIdleTime Done. https://codereview.chromium.org/1105293004/diff/100001/src/heap/gc-idle-time-... File src/heap/gc-idle-time-handler.h (right): https://codereview.chromium.org/1105293004/diff/100001/src/heap/gc-idle-time-... src/heap/gc-idle-time-handler.h:152: static const int kGCsBeforeMutatorIsActive = 7; On 2015/05/05 21:38:52, Erik Corry wrote: > These 7 GCs will normally be scavenges, so we are talking about 7x8Mbytes of > newspace allocation. Perhaps we could make this be in terms of Mbytes > allocated, not GCs. That way, if we experiment with different newspace sizes we > are not simultaneously making big changes to the heuristics around oldspace > collection timing. Good idea! We could replace the scavenges_ and mark_compacts_ counters with one allocation counter (in new and old space). Deriving that counter from the new-space size and the size of promoted objects is not straightforward. Let me think about that. https://codereview.chromium.org/1105293004/diff/100001/test/unittests/heap/gc... File test/unittests/heap/gc-idle-time-handler-unittest.cc (right): https://codereview.chromium.org/1105293004/diff/100001/test/unittests/heap/gc... test/unittests/heap/gc-idle-time-handler-unittest.cc:61: EXPECT_EQ(true, action.reduce_memory); On 2015/05/05 21:38:52, Erik Corry wrote: > EXPECT_TRUE(action.reduce_memory) Done. https://codereview.chromium.org/1105293004/diff/100001/test/unittests/heap/gc... test/unittests/heap/gc-idle-time-handler-unittest.cc:71: handler()->NotifyScavenge(); On 2015/05/05 21:38:52, Erik Corry wrote: > In this test there are 7 GCs in a row without any idle notification, but I don't > think that's realistic or what we want to test. Added call to Compute.
https://codereview.chromium.org/1105293004/diff/100001/src/heap/gc-idle-time-... File src/heap/gc-idle-time-handler.cc (right): https://codereview.chromium.org/1105293004/diff/100001/src/heap/gc-idle-time-... src/heap/gc-idle-time-handler.cc:199: // if can_start_incremental_marking is false. On 2015/05/06 09:47:19, ulan wrote: > On 2015/05/05 21:38:52, Erik Corry wrote: > > This contradiction indicates to me that can_start_incremental_marking is > > misnamed. > > Is should_start_incremental_marking better? I can rename it in a separate CL. Yes, we can do it in a different CL. I was thinking some name that explains why we can start incremental, since we can start it for other reasons when this flag is false. allocated_enough_to_start_incremental, perhaps. https://codereview.chromium.org/1105293004/diff/100001/src/heap/gc-idle-time-... src/heap/gc-idle-time-handler.cc:213: // kReduceMemory => kReduceLatency transition happens if N mutator GCs On 2015/05/06 09:47:19, ulan wrote: > On 2015/05/05 21:38:52, Erik Corry wrote: > > Should N be 1? If any allocation-provoked GCs are happening at all then the > > mutator is active and we should move to latency mode. > > This is to prevent back-and-forth transitions from latency mode to memory mode > if the mutator is almost idle: does a scavenge once in a while. I can see that makes sense. https://codereview.chromium.org/1105293004/diff/100001/src/heap/gc-idle-time-... File src/heap/gc-idle-time-handler.h (right): https://codereview.chromium.org/1105293004/diff/100001/src/heap/gc-idle-time-... src/heap/gc-idle-time-handler.h:152: static const int kGCsBeforeMutatorIsActive = 7; On 2015/05/06 09:47:19, ulan wrote: > On 2015/05/05 21:38:52, Erik Corry wrote: > > These 7 GCs will normally be scavenges, so we are talking about 7x8Mbytes of > > newspace allocation. Perhaps we could make this be in terms of Mbytes > > allocated, not GCs. That way, if we experiment with different newspace sizes > we > > are not simultaneously making big changes to the heuristics around oldspace > > collection timing. > > Good idea! We could replace the scavenges_ and mark_compacts_ counters with one > allocation counter (in new and old space). Deriving that counter from the > new-space size and the size of promoted objects is not straightforward. Let me > think about that. Perhaps set the limit to 50Mbytes, and every time we do a GC we increase the counter by the current size of a semispace. It's approximate, but roughly right, and pretty simple. https://codereview.chromium.org/1105293004/diff/120001/test/unittests/heap/gc... File test/unittests/heap/gc-idle-time-handler-unittest.cc (right): https://codereview.chromium.org/1105293004/diff/120001/test/unittests/heap/gc... test/unittests/heap/gc-idle-time-handler-unittest.cc:484: TransitionToReduceLatencyMode(heap_state); Either in this function or after this function we should test that we actually transitioned to reduce-latency mode. https://codereview.chromium.org/1105293004/diff/120001/test/unittests/heap/gc... test/unittests/heap/gc-idle-time-handler-unittest.cc:488: EXPECT_FALSE(action.reduce_memory); This does not check whether we are in reduce-latency mode.
https://codereview.chromium.org/1105293004/diff/100001/src/heap/gc-idle-time-... File src/heap/gc-idle-time-handler.h (right): https://codereview.chromium.org/1105293004/diff/100001/src/heap/gc-idle-time-... src/heap/gc-idle-time-handler.h:152: static const int kGCsBeforeMutatorIsActive = 7; On 2015/05/06 09:57:29, Erik Corry wrote: > On 2015/05/06 09:47:19, ulan wrote: > > On 2015/05/05 21:38:52, Erik Corry wrote: > > > These 7 GCs will normally be scavenges, so we are talking about 7x8Mbytes of > > > newspace allocation. Perhaps we could make this be in terms of Mbytes > > > allocated, not GCs. That way, if we experiment with different newspace > sizes > > we > > > are not simultaneously making big changes to the heuristics around oldspace > > > collection timing. > > > > Good idea! We could replace the scavenges_ and mark_compacts_ counters with > one > > allocation counter (in new and old space). Deriving that counter from the > > new-space size and the size of promoted objects is not straightforward. Let me > > think about that. > > Perhaps set the limit to 50Mbytes, and every time we do a GC we increase the > counter by the current size of a semispace. It's approximate, but roughly > right, and pretty simple. We know the promoted size, why not take this counter and accumulate?
https://codereview.chromium.org/1105293004/diff/100001/src/heap/gc-idle-time-... File src/heap/gc-idle-time-handler.cc (right): https://codereview.chromium.org/1105293004/diff/100001/src/heap/gc-idle-time-... src/heap/gc-idle-time-handler.cc:199: // if can_start_incremental_marking is false. On 2015/05/06 09:57:29, Erik Corry wrote: > On 2015/05/06 09:47:19, ulan wrote: > > On 2015/05/05 21:38:52, Erik Corry wrote: > > > This contradiction indicates to me that can_start_incremental_marking is > > > misnamed. > > > > Is should_start_incremental_marking better? I can rename it in a separate CL. > > Yes, we can do it in a different CL. I was thinking some name that explains why > we can start incremental, since we can start it for other reasons when this flag > is false. allocated_enough_to_start_incremental, perhaps. Acknowledged. https://codereview.chromium.org/1105293004/diff/100001/src/heap/gc-idle-time-... File src/heap/gc-idle-time-handler.h (right): https://codereview.chromium.org/1105293004/diff/100001/src/heap/gc-idle-time-... src/heap/gc-idle-time-handler.h:152: static const int kGCsBeforeMutatorIsActive = 7; On 2015/05/07 08:21:52, Hannes Payer wrote: > On 2015/05/06 09:57:29, Erik Corry wrote: > > On 2015/05/06 09:47:19, ulan wrote: > > > On 2015/05/05 21:38:52, Erik Corry wrote: > > > > These 7 GCs will normally be scavenges, so we are talking about 7x8Mbytes > of > > > > newspace allocation. Perhaps we could make this be in terms of Mbytes > > > > allocated, not GCs. That way, if we experiment with different newspace > > sizes > > > we > > > > are not simultaneously making big changes to the heuristics around > oldspace > > > > collection timing. > > > > > > Good idea! We could replace the scavenges_ and mark_compacts_ counters with > > one > > > allocation counter (in new and old space). Deriving that counter from the > > > new-space size and the size of promoted objects is not straightforward. Let > me > > > think about that. > > > > Perhaps set the limit to 50Mbytes, and every time we do a GC we increase the > > counter by the current size of a semispace. It's approximate, but roughly > > right, and pretty simple. > > We know the promoted size, why not take this counter and accumulate? We can either use new space committed size, new space live size, or promoted size. I will experiments to see what is better. Erik, Hannes, are ok with landing this CL now to make experimenting easier? https://codereview.chromium.org/1105293004/diff/120001/test/unittests/heap/gc... File test/unittests/heap/gc-idle-time-handler-unittest.cc (right): https://codereview.chromium.org/1105293004/diff/120001/test/unittests/heap/gc... test/unittests/heap/gc-idle-time-handler-unittest.cc:484: TransitionToReduceLatencyMode(heap_state); On 2015/05/06 09:57:29, Erik Corry wrote: > Either in this function or after this function we should test that we actually > transitioned to reduce-latency mode. Done. https://codereview.chromium.org/1105293004/diff/120001/test/unittests/heap/gc... test/unittests/heap/gc-idle-time-handler-unittest.cc:488: EXPECT_FALSE(action.reduce_memory); On 2015/05/06 09:57:29, Erik Corry wrote: > This does not check whether we are in reduce-latency mode. Done.
lgtm
The CQ bit was checked by ulan@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from hpayer@chromium.org Link to the patchset: https://codereview.chromium.org/1105293004/#ps140001 (title: "Add checks in tests")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1105293004/140001
Message was sent while issue was closed.
Committed patchset #8 (id:140001)
Message was sent while issue was closed.
Patchset 8 (id:??) landed as https://crrev.com/ae6a0b807574d4349be9c45428960ac7ecd3679a Cr-Commit-Position: refs/heads/master@{#28300}
Message was sent while issue was closed.
This is in Canary 2396 It is referenced from https://code.google.com/p/chromium/issues/detail?id=486005 |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
