Bug for investigating memory regression on context (maybe related to Array Iterator redesign) |
|||||||
Issue descriptionLet's see
,
Mar 6 2018
📍 Pinpoint job started. https://pinpoint-dot-chromeperf.appspot.com/job/14b24e62440000
,
Mar 7 2018
📍 Found significant differences after each of 33 commits. https://pinpoint-dot-chromeperf.appspot.com/job/14b24e62440000 Roll V8 back to 6.6.346. by grt@chromium.org https://chromium.googlesource.com/chromium/src/+/1808b6997fe1e50153125f054a6589863733b5bf [builtins] Remove the ObjectConstructor_ConstructStub builtin. by bmeurer@chromium.org https://chromium.googlesource.com/v8/v8/+/c8f34835af9168ad75c43d79da070fd541106040 [builtins] Remove BooleanConstructor_ConstructStub builtin by mathias@chromium.org https://chromium.googlesource.com/v8/v8/+/df5483caeee99df7df4a84da385a9b70440a19e5 [builtins] Remove BigIntConstructor_ConstructStub builtin by mathias@chromium.org https://chromium.googlesource.com/v8/v8/+/306f2fd5a78ba596ab32e8478c2a0f5833a8b295 [builtins] Remove ArrayBufferConstructor_ConstructStub builtin by mathias@chromium.org https://chromium.googlesource.com/v8/v8/+/f8c688f4c26c8186788026ff26dfb9dffdf3be6e [builtins] Remove DateConstructor_ConstructStub builtin. by bmeurer@chromium.org https://chromium.googlesource.com/v8/v8/+/251f63d82f82f1f67393566e5757a162821b6262 [builtins] Remove SymbolConstructor_ConstructStub builtin by mathias@chromium.org https://chromium.googlesource.com/v8/v8/+/0ffaedf8d55861059d47ddcb037b0117369ae321 [builtins] Remove DataViewConstructor_ConstructStub builtin by mathias@chromium.org https://chromium.googlesource.com/v8/v8/+/fa82d01ed06c59800f23631487927e17f817b415 Fix buffer-detached check in TypedArray.of/from by jkummerow@chromium.org https://chromium.googlesource.com/v8/v8/+/c94df3cec425d6fda250a19b3cc75ef924529e3d [x64] Use XOR instruction to zero register in SpeculationPoison by kanghua.yu@intel.com https://chromium.googlesource.com/v8/v8/+/a4b615eba1ae2963b0c5659ca22ba42bf16c09bd [builtins] Refactor the StringConstructor builtin by mathias@chromium.org https://chromium.googlesource.com/v8/v8/+/15e207b300bf7f7b70df83e98cbaf8c0d99a7485 [runtime] Move validity cell from PrototypeInfo to Map. by ishell@chromium.org https://chromium.googlesource.com/v8/v8/+/40a3e6dcb9bdb5cb2a7f6e29fa8e4de75e538ef6 [wasm] Turn {WasmDebugInfo} into a proper {Struct}. by mstarzinger@chromium.org https://chromium.googlesource.com/v8/v8/+/67fa841bcb1e43977f24917d4b57be2284c6e8e5 Reland "[parser] Remove pretenuring of closures assigned to properties" by adamk@chromium.org https://chromium.googlesource.com/v8/v8/+/3d7ad2e7e53d3cd6add51512580c054abf7e5805 [parsing] inline ArrayLiteral creation for spread calls by caitp@igalia.com https://chromium.googlesource.com/v8/v8/+/93fc3841c3da0fc85662e66a57d881555238d634 [runtime] Always store the name in a function's ScopeInfo by cbruni@chromium.org https://chromium.googlesource.com/v8/v8/+/01488b9c4f1b4c8e2b66494b24d7e7ff8a826860 Revert "[parsing] inline ArrayLiteral creation for spread calls" by sigurds@chromium.org https://chromium.googlesource.com/v8/v8/+/f48e7349035430ea13dd0ce1ed1e775f6eecabeb [es2015] Extend the array iterator protector. by bmeurer@chromium.org https://chromium.googlesource.com/v8/v8/+/1525374ff5a564b55b748ad33e6cd0d0ea684006 Reland "[parsing] inline ArrayLiteral creation for spread calls" by neis@chromium.org https://chromium.googlesource.com/v8/v8/+/82345e9fbfb4f6b0177c19eb476cc6c6babcef82 [wasm] Turn {WasmSharedModuleData} into a proper {Struct}. by mstarzinger@chromium.org https://chromium.googlesource.com/v8/v8/+/d623fcae9cad5cb56475b7f1065a0a1adc2cf432 [builtins] Refactor the NumberConstructor builtin by mathias@chromium.org https://chromium.googlesource.com/v8/v8/+/279085d852cb0c90c19fab73e9b464f354c7d707 [callbacks] Clean up PropertyCallbackArguments by cbruni@chromium.org https://chromium.googlesource.com/v8/v8/+/bb862bbc504db5a296ced8e057923cc1d83606f8 [runtime] Add BOILERPLATE_DESCRIPTION_TYPE InstanceType by cbruni@chromium.org https://chromium.googlesource.com/v8/v8/+/1f307ba52fa3a7f820be554f7212b7b302b49b62 [ic] Remove pointless macros and obsolete comments. by neis@chromium.org https://chromium.googlesource.com/v8/v8/+/97b3a968a518baf98e93a0f399a579f9128b8e31 [builtins] Refactor the ProxyConstructor builtin by mathias@chromium.org https://chromium.googlesource.com/v8/v8/+/08e168d0ff38f97f8638d25fdd050dda7ae10ca6 [ic] Introduce new IC for storing into array literals. by neis@chromium.org https://chromium.googlesource.com/v8/v8/+/2e2860f74f9c610d5ffb3ee1778d937055d25dea [builtins] Enable embedded builtins and add testing variants by jgruber@chromium.org https://chromium.googlesource.com/v8/v8/+/abcc28ced071faee637d5fd59d06cfdf80d8ea3f [ic] Relax a CHECK. by neis@chromium.org https://chromium.googlesource.com/v8/v8/+/c895a23a996beafcd706e18656c8673338167ebd [turbofan] Fix bug in Array.p.reduceRight by sigurds@chromium.org https://chromium.googlesource.com/v8/v8/+/d1df563059107761c21d9c587169302644e906de [interpreter] Only create spread-related feedback slots when necessary. by neis@chromium.org https://chromium.googlesource.com/v8/v8/+/cf8cd1c444e7975211915fc77c52fe3c66876f78 [in-place weak refs] Add in-place weak references & migrate one WeakCell to it. by marja@chromium.org https://chromium.googlesource.com/v8/v8/+/07c1e641d9864cb2de9692cf69bccca4b49a6f6a Revert "[in-place weak refs] Add in-place weak references & migrate one WeakCell to it." by sigurds@chromium.org https://chromium.googlesource.com/v8/v8/+/73d6037c2051816fe926ae127aa52e863a800189 [es2015] Refactor the JSArrayIterator. by bmeurer@chromium.org https://chromium.googlesource.com/v8/v8/+/06ee127b75726f9ee541aab10f6aecfe4d96675a Understanding performance regressions: http://g.co/ChromePerformanceRegressions
,
Mar 7 2018
<dtu from Pinpoint team> looks like all of these commits caused an improvement or regression in memory, but most of them were very small. The biggest regressions were at: [runtime] Move validity cell from PrototypeInfo to Map. by ishell@chromium.org https://chromium.googlesource.com/v8/v8/+/40a3e6dcb9bdb5cb2a7f6e29fa8e4de75e538ef6 [builtins] Enable embedded builtins and add testing variants by jgruber@chromium.org https://chromium.googlesource.com/v8/v8/+/abcc28ced071faee637d5fd59d06cfdf80d8ea3f I hope to have the regression sizes in the bug comment too (go/catabug/3871) And only CC the authors/reviewers of the biggest changes (go/catabug/4317) Also +perezju, the benchmark owner.
,
Mar 8 2018
[builtins] Enable embedded builtins and add testing variants From the pinpoint job, it looks like this increases memory usage (memory:chrome:all_processes:reported_by_chrome:v8:heap:allocated_objects_size) by roughly 20K. AFAIK there's two sources of memory regressions in this CL: 1. It introduces the constants list containing all constants and external references which were previously embedded into builtins. The constants list is a FixedArray on the root list. 2. Builtins now access constants and external refs through an indirection, which produces a longer instruction sequence than directly embedded constants. An increase by 20K is not unexpected, and once builtins are entirely off-heap, the memory savings should outweigh these regressions by far. Alternatively, we still have the option to unship embedded builtins if they are not ready for 67. bmeurer@, your CL ([es2015] Refactor the JSArrayIterator) reduces memory consumption by roughly 10k. ishell@, your CL ([runtime] Move validity cell from PrototypeInfo to Map.) increases it by <10k. Assigning back to bmeurer@.
,
Mar 8 2018
,
Mar 8 2018
FYI: this CL [runtime] Move validity cell from PrototypeInfo to Map. by ishell@chromium.org https://chromium.googlesource.com/v8/v8/+/40a3e6dcb9bdb5cb2a7f6e29fa8e4de75e538ef6 is expected to regress memory. The regression will be partially recovered once the next CL lands (https://chromium-review.googlesource.com/c/v8/v8/+/931701).
,
Mar 8 2018
As determined by jgruber@ my CL improves memory. Assigning to memory sheriff for investigation.
,
Mar 8 2018
There are two large (for this range) regressions and one large improvement. The two large regressions mentioned by @dtu cover the vast majority of the increase across this range, so as long as jgruber and ishell have a plan to address them, there is no action needed on this bug. See the picture attached for a graphical representation :) |
|||||||
►
Sign in to add a comment |
|||||||
Comment 1 by bewhetst...@gmail.com
, Mar 6 2018