- 27 Apr, 2017 1 commit
-
-
cbruni authored
With this CL we reduce the difference between directly using a null prototype in a literal or using Object.create(null). - The EmitFastCloneShallowObject builtin now supports cloning slow object boilerplates. - Unified behavior to find the matching Map and instantiating it for Object.create(null) and literals with a null prototype. - Cleanup of literal type parameter of CompileTimeValue, now in sync with ObjectLiteral flags. Review-Url: https://codereview.chromium.org/2445333002 Cr-Commit-Position: refs/heads/master@{#44941}
-
- 26 Apr, 2017 3 commits
-
-
Adam Klein authored
Clearing out the constructor field is invalid in the case where the function's map has transitioned since the last SetPrototype call. Bug: chromium:714972 Change-Id: Ie918702a128219c4995b805f7c9a53b41cc4e4b6 Reviewed-on: https://chromium-review.googlesource.com/486130 Commit-Queue: Adam Klein <adamk@chromium.org> Reviewed-by:
Toon Verwaest <verwaest@chromium.org> Cr-Commit-Position: refs/heads/master@{#44906}
-
Adam Klein authored
This allows us to avoid a separate receiver typecheck in a few places without regressing the error messages generated. As more Array methods move to C++, this will get more usage. Bug: v8:3577 Change-Id: Ibdd17c781548520172ce62442bc3a800e5c09e99 Reviewed-on: https://chromium-review.googlesource.com/486103Reviewed-by:
Sathya Gunasekaran <gsathya@chromium.org> Commit-Queue: Adam Klein <adamk@chromium.org> Cr-Commit-Position: refs/heads/master@{#44904}
-
neis authored
- Use Assembler in a few places that unneccessarily used MacroAssembler before. - Fix some comments. R=jarin@chromium.org BUG=v8:6048 Review-Url: https://codereview.chromium.org/2843933002 Cr-Commit-Position: refs/heads/master@{#44894}
-
- 25 Apr, 2017 6 commits
-
-
Michael Achenbach authored
This reverts commit 28930128. Reason for revert: GC stress failures: https://build.chromium.org/p/client.v8/builders/V8%20Mac%20GC%20Stress/builds/12958 Original change's description: > [runtime] Keep FAST_SLOPPY_ARGUMENTS packed > > With this CL SloppyArguments immediately go to dictionary elements on > deletion, keeping the arguments backing store packed. > > Bug: v8:6251 > Change-Id: I2afa4fb5f0af9942eee0a1606942f5f289539330 > Reviewed-on: https://chromium-review.googlesource.com/480379 > Commit-Queue: Camillo Bruni <cbruni@chromium.org> > Reviewed-by: Jakob Kummerow <jkummerow@chromium.org> > Cr-Commit-Position: refs/heads/master@{#44857} TBR=jkummerow@chromium.org,cbruni@chromium.org,v8-reviews@googlegroups.com NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true Change-Id: I9482bf693a745d1301d068869ddae39f11143827 Reviewed-on: https://chromium-review.googlesource.com/486885Reviewed-by:
Michael Achenbach <machenbach@chromium.org> Commit-Queue: Michael Achenbach <machenbach@chromium.org> Cr-Commit-Position: refs/heads/master@{#44863}
-
ulan authored
This makes an ObjectVisitor as powerful as a StaticVisitor and allows slots recording in ObjectVisitor. This patch also renames VisitCell method of ObjectVisitor to VisitCellPointer, so that VisitCell is free to be used for actually visiting a cell. BUG=chromium:709075 Review-Url: https://codereview.chromium.org/2810653002 Cr-Commit-Position: refs/heads/master@{#44860}
-
Camillo Bruni authored
With this CL SloppyArguments immediately go to dictionary elements on deletion, keeping the arguments backing store packed. Bug: v8:6251 Change-Id: I2afa4fb5f0af9942eee0a1606942f5f289539330 Reviewed-on: https://chromium-review.googlesource.com/480379 Commit-Queue: Camillo Bruni <cbruni@chromium.org> Reviewed-by:
Jakob Kummerow <jkummerow@chromium.org> Cr-Commit-Position: refs/heads/master@{#44857}
-
ulan authored
This patch adds a new interface called RootVisitor and changes the root iteration functions to accept a RootVisitor instead of an ObjectVisitor. Future CLs will change ObjectVisitor to provide the host object to all visiting functions, which will bring it in sync with static visitors. Having separate visitors for roots and objects removes ambiguity in VisitPointers and reduces chances of forgetting to record slots. This is intended as pure refactoring. All places that require behavior change are marked with TODO and will addressed in future CLs. BUG=chromium:709075 Review-Url: https://codereview.chromium.org/2801073006 Cr-Commit-Position: refs/heads/master@{#44852}
-
Marja Hölttä authored
The data produced by the preparser scope analysis might be large. ByteArrays are already allowed in the large object space. This fixes mjsunit/asm/poppler/poppler.js with the flag on. First version landed as https://chromium-review.googlesource.com/c/484459/ this version includes gen-postmortem-metadata fixes. BUG=v8:5516 Change-Id: I2218c4729ba9feefd6595a93e5cc6d2e52ebda0e Reviewed-on: https://chromium-review.googlesource.com/486641Reviewed-by:
Yang Guo <yangguo@chromium.org> Reviewed-by:
Michael Starzinger <mstarzinger@chromium.org> Commit-Queue: Marja Hölttä <marja@chromium.org> Cr-Commit-Position: refs/heads/master@{#44835}
-
jgruber authored
Do not bail out when passed a flags string with length > 5, use a meaningful named constant instead. Found by https://github.com/tc39/test262/pull/997#issuecomment-296963675 BUG=v8:6300 Review-Url: https://codereview.chromium.org/2841633004 Cr-Commit-Position: refs/heads/master@{#44834}
-
- 24 Apr, 2017 6 commits
-
-
Adam Klein authored
This patch removes JSFunction::SetInstancePrototype() from JSFunction's public API and makes it an implementation detail of SetPrototype(). Also clear out constructor field of JSFunction Map when transitioning from non-instance prototype to instance prototype. Change-Id: If51d37bf6047b51b934d1b370fb52bb5cf5ffed4 Reviewed-on: https://chromium-review.googlesource.com/483961Reviewed-by:
Toon Verwaest <verwaest@chromium.org> Reviewed-by:
Brad Nelson <bradnelson@chromium.org> Commit-Queue: Adam Klein <adamk@chromium.org> Cr-Commit-Position: refs/heads/master@{#44821}
-
jkummerow authored
In general, deleting a property from a fast-properties object requires transitioning the object to dictionary mode. However, when the most-recently-added property is deleted, we can simply roll back the last map transition that the object went through. This is a performance experiment: it should make things faster, but if it turns out to have more negative than positive impact, we will have to revert it. TBR=bmeurer@chromium.org (just adding a comment) Previously reviewed at https://codereview.chromium.org/2830093002 Previously landed as 98acfb36 / r44799 Review-Url: https://codereview.chromium.org/2840583002 Cr-Commit-Position: refs/heads/master@{#44808}
-
machenbach authored
Revert of [builtins] DeleteProperty: Handle last-added fast properties (patchset #2 id:20001 of https://codereview.chromium.org/2830093002/ ) Reason for revert: Breaks: https://build.chromium.org/p/client.v8/builders/V8%20Mac%20GC%20Stress/builds/12920 and https://build.chromium.org/p/client.v8/builders/V8%20Linux%20-%20gc%20stress/builds/10281 Original issue's description: > [builtins] DeleteProperty: Handle last-added fast properties > > In general, deleting a property from a fast-properties object > requires transitioning the object to dictionary mode. However, > when the most-recently-added property is deleted, we can simply > roll back the last map transition that the object went through. > > This is a performance experiment: it should make things faster, > but if it turns out to have more negative than positive impact, > we will have to revert it. > > TBR=bmeurer@chromium.org (just adding a comment) > > Review-Url: https://codereview.chromium.org/2830093002 > Cr-Commit-Position: refs/heads/master@{#44799} > Committed: https://chromium.googlesource.com/v8/v8/+/98acfb36e1acf2ab52ab6b6439eb6356c83dcda6 TBR=ishell@chromium.org,jkummerow@chromium.org # Skipping CQ checks because original CL landed less than 1 days ago. NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true Review-Url: https://codereview.chromium.org/2843473002 Cr-Commit-Position: refs/heads/master@{#44806}
-
Franziska Hinkelmann authored
This reverts commit e8f1fc24. Reason for revert: Node.js doesn't build with this patch anymore. out/Release/obj/gen/debug-support.cc:428:55: error: expected initializer before ‘<’ token int v8dbg_class_Script__preparsed_scope_data__PodArray<uint32_t> = Script::kPreParsedScopeDataOffset; Original change's description: > [parser] Skipping inner funcs: use PodArray for the data. > > The data produced by the preparser scope analysis might be large. > > ByteArrays are already allowed in the large object space. > > This fixes mjsunit/asm/poppler/poppler.js with the flag on. > > BUG=v8:5516 > > Change-Id: I951836244776c57efdd2a491c5c78493dc8cca63 > Reviewed-on: https://chromium-review.googlesource.com/484459 > Commit-Queue: Marja Hölttä <marja@chromium.org> > Reviewed-by: Michael Starzinger <mstarzinger@chromium.org> > Cr-Commit-Position: refs/heads/master@{#44795} TBR=marja@chromium.org,mstarzinger@chromium.org,ulan@chromium.org,vogelheim@chromium.org,hpayer@chromium.org,v8-reviews@googlegroups.com NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG=v8:5516 Change-Id: I3012d27b6b65b37d3afc5f3b0921e044bdcc118e Reviewed-on: https://chromium-review.googlesource.com/485759Reviewed-by:
Franziska Hinkelmann <franzih@chromium.org> Commit-Queue: Franziska Hinkelmann <franzih@chromium.org> Cr-Commit-Position: refs/heads/master@{#44805}
-
jkummerow authored
In general, deleting a property from a fast-properties object requires transitioning the object to dictionary mode. However, when the most-recently-added property is deleted, we can simply roll back the last map transition that the object went through. This is a performance experiment: it should make things faster, but if it turns out to have more negative than positive impact, we will have to revert it. TBR=bmeurer@chromium.org (just adding a comment) Review-Url: https://codereview.chromium.org/2830093002 Cr-Commit-Position: refs/heads/master@{#44799}
-
Marja Hölttä authored
The data produced by the preparser scope analysis might be large. ByteArrays are already allowed in the large object space. This fixes mjsunit/asm/poppler/poppler.js with the flag on. BUG=v8:5516 Change-Id: I951836244776c57efdd2a491c5c78493dc8cca63 Reviewed-on: https://chromium-review.googlesource.com/484459 Commit-Queue: Marja Hölttä <marja@chromium.org> Reviewed-by:
Michael Starzinger <mstarzinger@chromium.org> Cr-Commit-Position: refs/heads/master@{#44795}
-
- 21 Apr, 2017 3 commits
-
-
jkummerow authored
Review-Url: https://codereview.chromium.org/2827263004 Cr-Commit-Position: refs/heads/master@{#44783}
-
Sathya Gunasekaran authored
This reverts commit 64c12860. Reason for revert: the number of buckets can be changed later, so we can't cache the value Original change's description: > [OrderedHashSet] Remove extra entry lookup > > Bug: v8:5717 > Change-Id: I999ea81f745a25a1f3a539e50594e597e64f5287 > Reviewed-on: https://chromium-review.googlesource.com/481044 > Reviewed-by: Adam Klein <adamk@chromium.org> > Commit-Queue: Sathya Gunasekaran <gsathya@chromium.org> > Cr-Commit-Position: refs/heads/master@{#44705} TBR=adamk@chromium.org,cbruni@chromium.org,gsathya@chromium.org,v8-reviews@googlegroups.com # Not skipping CQ checks because original CL landed > 1 day ago. Change-Id: Iade35c0041e943f6dedb24844b07034f56ff1150 Reviewed-on: https://chromium-review.googlesource.com/483904 Commit-Queue: Sathya Gunasekaran <gsathya@chromium.org> Reviewed-by:
Sathya Gunasekaran <gsathya@chromium.org> Reviewed-by:
Adam Klein <adamk@chromium.org> Cr-Commit-Position: refs/heads/master@{#44761}
-
Sathya Gunasekaran authored
This reverts commit 88376a78. Reason for revert: the number of buckets could change, so we can't cache the value Original change's description: > [OrderedHashSet] Remove redundant hash to bucket > > Bug: v8:5717 > Change-Id: I88e1e3089844b0955f0f7a6a792c2e10949a5b18 > Reviewed-on: https://chromium-review.googlesource.com/480927 > Reviewed-by: Adam Klein <adamk@chromium.org> > Commit-Queue: Sathya Gunasekaran <gsathya@chromium.org> > Cr-Commit-Position: refs/heads/master@{#44707} TBR=adamk@chromium.org,cbruni@chromium.org,gsathya@chromium.org,v8-reviews@googlegroups.com # Not skipping CQ checks because original CL landed > 1 day ago. Change-Id: I4d42021a1d1f5bc069b3787efc828761098038e0 Reviewed-on: https://chromium-review.googlesource.com/483903Reviewed-by:
Sathya Gunasekaran <gsathya@chromium.org> Reviewed-by:
Adam Klein <adamk@chromium.org> Commit-Queue: Adam Klein <adamk@chromium.org> Cr-Commit-Position: refs/heads/master@{#44760}
-
- 20 Apr, 2017 3 commits
-
-
Adam Klein authored
It was a straight pass-through to JSFunction::SetPrototype, with the added wrinkle that it appeared to sometimes throw (although it never did). Also improves typing of JSFunction::SetInstancePrototype signature to require being passed a JSReceiver. Change-Id: Ie85b9a74955f72bf988cd902c5eec34e32b51a24 Reviewed-on: https://chromium-review.googlesource.com/482421Reviewed-by:
Toon Verwaest <verwaest@chromium.org> Reviewed-by:
Brad Nelson <bradnelson@chromium.org> Commit-Queue: Adam Klein <adamk@chromium.org> Cr-Commit-Position: refs/heads/master@{#44757}
-
Michael Achenbach authored
This reverts commit 64bb6e6c. Reason for revert: Breaks layout tests: https://build.chromium.org/p/client.v8.fyi/builders/V8-Blink%20Linux%2064/builds/15092 See: https://github.com/v8/v8/wiki/Blink-layout-tests Original change's description: > [runtime] Pass global proxy as receiver to native accessors in case of contextual access > > Bug: > > Change-Id: I288c0d7a34b65eda6c6e46168c436b87a350f6d4 > Reviewed-on: https://chromium-review.googlesource.com/483199 > Commit-Queue: Toon Verwaest <verwaest@chromium.org> > Reviewed-by: Yang Guo <yangguo@chromium.org> > Cr-Commit-Position: refs/heads/master@{#44739} TBR=yangguo@chromium.org,verwaest@chromium.org,v8-reviews@googlegroups.com NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true Change-Id: Ifc204ce5a2e6d774b993210fcc6782fc6f27dd7b Reviewed-on: https://chromium-review.googlesource.com/483480Reviewed-by:
Michael Achenbach <machenbach@chromium.org> Commit-Queue: Michael Achenbach <machenbach@chromium.org> Cr-Commit-Position: refs/heads/master@{#44743}
-
Toon Verwaest authored
Bug: Change-Id: I288c0d7a34b65eda6c6e46168c436b87a350f6d4 Reviewed-on: https://chromium-review.googlesource.com/483199 Commit-Queue: Toon Verwaest <verwaest@chromium.org> Reviewed-by:
Yang Guo <yangguo@chromium.org> Cr-Commit-Position: refs/heads/master@{#44739}
-
- 19 Apr, 2017 1 commit
-
-
neis authored
When asked for a module that previously failed to compile or instantiate, the embedder necessarily has to signal failure. In this case, we expect an exception to be scheduled, which we will rethrow. BUG=v8:1569 Review-Url: https://codereview.chromium.org/2827733002 Cr-Commit-Position: refs/heads/master@{#44729}
-
- 18 Apr, 2017 4 commits
-
-
Sathya Gunasekaran authored
Bug: v8:5717 Change-Id: I88e1e3089844b0955f0f7a6a792c2e10949a5b18 Reviewed-on: https://chromium-review.googlesource.com/480927Reviewed-by:
Adam Klein <adamk@chromium.org> Commit-Queue: Sathya Gunasekaran <gsathya@chromium.org> Cr-Commit-Position: refs/heads/master@{#44707}
-
Sathya Gunasekaran authored
Bug: v8:5717 Change-Id: I999ea81f745a25a1f3a539e50594e597e64f5287 Reviewed-on: https://chromium-review.googlesource.com/481044Reviewed-by:
Adam Klein <adamk@chromium.org> Commit-Queue: Sathya Gunasekaran <gsathya@chromium.org> Cr-Commit-Position: refs/heads/master@{#44705}
-
yangguo authored
We can use TUPLE2 or TUPLE3 for structs that do not need special handling by deoptimizer and compiler. This frees up a few instance types, so that adding the next few new structs will not cause ABI compatibility to break. R=mstarzinger@chromium.org Review-Url: https://codereview.chromium.org/2811183005 Cr-Commit-Position: refs/heads/master@{#44685}
-
Marja Hölttä authored
No usage sites are getting the length for uncompiled functions, so we can postpone setting the correct length until after compilation. This way we don't need to produce and store it for skipped inner functions. In the current implementation, getting the function length compiles it (and users rely on it - so the feature is probably not going to go away). BUG=v8:5516 Change-Id: Id8c9a05d2391505a6cde613841094170c9a1b808 Reviewed-on: https://chromium-review.googlesource.com/468927 Commit-Queue: Marja Hölttä <marja@chromium.org> Reviewed-by:
Michael Starzinger <mstarzinger@chromium.org> Reviewed-by:
Daniel Vogelheim <vogelheim@chromium.org> Cr-Commit-Position: refs/heads/master@{#44679}
-
- 13 Apr, 2017 3 commits
-
-
jkummerow authored
TBR=ishell@chromium.org NOTRY=true Review-Url: https://codereview.chromium.org/2818773002 Cr-Commit-Position: refs/heads/master@{#44651}
-
jkummerow authored
Taking the slow runtime path for every non-internalized string key can be avoided by doing optimistic string table lookups: if there is a matching entry, use that; if there isn't, then no existing object has a property with that name. The hashing/internalizing logic is in C++ and called directly. Review-Url: https://codereview.chromium.org/2811333002 Cr-Commit-Position: refs/heads/master@{#44650}
-
Leszek Swirski authored
Currently we count optimizations to decide to disable optimization, and count deopts to detect this decision and allow re-enabling optimizations after a while. However, throwing out TurboFan OSR code and GC optimized code evictions do not count as deopts, which means that the optimization count increases without increasing the deopt count. This increased optimization count disables further optimization -- which is bad, because these are not "true" deopts -- and can stop the optimization from being re-enabled, because the deopt count can't go high enough. Instead, we now only ever look at deopts to disable/re-enable optimization, and opt counts are only used for naming log files and in tests. Change-Id: I0c7d6be497545449a38cf952cd2f007ee51982ba Reviewed-on: https://chromium-review.googlesource.com/468811 Commit-Queue: Leszek Swirski <leszeks@chromium.org> Reviewed-by:
Ross McIlroy <rmcilroy@chromium.org> Reviewed-by:
Jaroslav Sevcik <jarin@chromium.org> Reviewed-by:
Benedikt Meurer <bmeurer@chromium.org> Reviewed-by:
Michael Starzinger <mstarzinger@chromium.org> Cr-Commit-Position: refs/heads/master@{#44647}
-
- 12 Apr, 2017 3 commits
-
-
kozyatinskiy authored
Usually program doesn't contain a lot of different stack frames in collected stack trace. BUG=v8:6189 R=yangguo@chromium.orr TBR=bmeurer@chromium.org Review-Url: https://codereview.chromium.org/2788413004 Cr-Commit-Position: refs/heads/master@{#44622}
-
jgruber authored
This adds a fast path to skip runtime calls to GetSubstitution when the replacer string does not contain a '$' char. Extended background: String.prototype.replace is (roughly) structured as follows: * Check if {searchValue} has a @@replace Symbol, and delegate to that if so. We currently implement efficient fast paths when {searchValue} is a String or a fast RegExp. * A specialized fast path for single-char {searchValue}, "long" subject string, and String {replaceValue} that do not contain '$' chars (yes, this fast path is very specialized). * Check for the location of the first match using StringIndexOf, and exit early if no match is found. * Finally build the return value, which is 'prefix + replacement + suffix', where replacement is either the result of calling {replaceValue} (if it is callable), or GetSubstitution(ToString({replaceValue})) otherwise. There's several spots that could be improved. StringIndexOf currently calls into C++ runtime for all but the simple 1-byte, 1-char {searchValue} case. We need to finally add support for remaining cases. The runtime call to GetSubstitution can be skipped if the replacer string does not contain any '$' syntax. This CL handles that case. BUG= Review-Url: https://codereview.chromium.org/2813843002 Cr-Commit-Position: refs/heads/master@{#44606}
-
bmeurer authored
The hole NaN should also have proper Type::Hole, and not silently hide in the Type::Number. This way we can remove all the special casing for the hole NaN, and we also finally get the CheckNumber right. This also allows us to remove some ducktape from the Deoptimizer, as for escape analyzed FixedDoubleArrays we always pass the hole value now to represent the actual holes. Also-By: jarin@chromium.org BUG=chromium:684208,chromium:709753,v8:5267 R=jarin@chromium.org Review-Url: https://codereview.chromium.org/2814013003 Cr-Commit-Position: refs/heads/master@{#44603}
-
- 10 Apr, 2017 2 commits
-
-
Peter Marshall authored
We assumed that every JSArray would have a JSObject as a prototype, but it could be null, in which case we bail out to slow path. Also rename spread_array variable here, because this fast-path isn't just used by spreads anymore. Bug: chromium:707675 Change-Id: I8045d83977735dd00c3ebde2e0704f6b04afdedd Reviewed-on: https://chromium-review.googlesource.com/472907Reviewed-by:
Michael Starzinger <mstarzinger@chromium.org> Commit-Queue: Peter Marshall <petermarshall@chromium.org> Cr-Commit-Position: refs/heads/master@{#44531}
-
Marja Hölttä authored
The DCHECK added by https://chromium-review.googlesource.com/461827 was not true in case we failed to compile the function. BUG=chromium:708598 Change-Id: I6a542c3ac6281c0549396b4ff0af34ea44450006 Reviewed-on: https://chromium-review.googlesource.com/472826Reviewed-by:
Michael Starzinger <mstarzinger@chromium.org> Commit-Queue: Marja Hölttä <marja@chromium.org> Cr-Commit-Position: refs/heads/master@{#44513}
-
- 07 Apr, 2017 1 commit
-
-
jgruber authored
References to invalid names (i.e. not specified as a named group in the pattern) throw a SyntaxError. Unmatched groups are still replaced by the empty string. See https://github.com/tc39/proposal-regexp-named-groups/issues/14. BUG=v8:5437 Review-Url: https://codereview.chromium.org/2791183002 Cr-Commit-Position: refs/heads/master@{#44471}
-
- 06 Apr, 2017 2 commits
-
-
Marja Hölttä authored
BUG=v8:5402 R=mstarzinger@chromium.org Change-Id: I8ce43504fee83dcb6859418a526b2c7aea52e778 Reviewed-on: https://chromium-review.googlesource.com/468968 Commit-Queue: Marja Hölttä <marja@chromium.org> Reviewed-by:
Michael Starzinger <mstarzinger@chromium.org> Cr-Commit-Position: refs/heads/master@{#44436}
-
Camillo Bruni authored
This CL introduces SloppyArgumentsElements to encapsulate all the constants for SLOW_ and FAST_SLOPPY_ARGUMENTS_KINDS. This will serve as a better documentation and reduces the use of undocumented constants. Change-Id: I7a5b4e79f02573161d8a83aaf6f69fc490883aa5 Reviewed-on: https://chromium-review.googlesource.com/467666Reviewed-by:
Jakob Kummerow <jkummerow@chromium.org> Reviewed-by:
Igor Sheludko <ishell@chromium.org> Commit-Queue: Camillo Bruni <cbruni@chromium.org> Cr-Commit-Position: refs/heads/master@{#44433}
-
- 05 Apr, 2017 1 commit
-
-
Marja Hölttä authored
There's no need to set it so early - it's only needed when the function has really been parsed. This way we don't need to produce and store it for skipped inner functions. BUG=v8:5516 Change-Id: Ibf59a8acb886ea3de9be140431a334a03b408f5b Reviewed-on: https://chromium-review.googlesource.com/461827 Commit-Queue: Marja Hölttä <marja@chromium.org> Reviewed-by:
Daniel Vogelheim <vogelheim@chromium.org> Reviewed-by:
Michael Starzinger <mstarzinger@chromium.org> Cr-Commit-Position: refs/heads/master@{#44410}
-
- 04 Apr, 2017 1 commit
-
-
kschimpf authored
After discussion with Chrome reviewers for UMA, it was decided that we would report array buffer allocation sizes in megabytes (not the log). They also wanted to wait until there is proof that small array buffer allocations would flood the histogram. Hence, all allocation sizes are sampled. There were several ways we could have added the notion of megabyte samples to V8 code. None of them are a great fit. This code simply provides a local function within the code that needs it. Other possible solutions but rejected were: a) Use a subclass of histogram to collect data at the megabyte level. It has it's own Add() method that converts the size from bytes to megabytes, and then call the generic add method AddSample(). This solution appears to follow the conventions of subclasses of class Histogram. b) Use Chrome macros - Rejected because it involves changing the counter representation of V8. c) Add a method AddMegabyteSample() to base class Histogram. Rejected because it may get confusing if a lot of different measures are added the the base class of histograms. d) Make method AddSample() virtual and override in the derived class. Rejected in that sampling is supposed to be fast, and adding a virtual call may be breaking that contract. d) Do not add a derived class. Rather just do the conversions at the call sites. Rejected because this duplicates code, and also makes it hard to change assumptions on how to calculate. For Chromes UMA changes see: CL: https://codereview.chromium.org/2795463002 BUG=chromium:704922 R=bbudge@chromium.org,bradnelson@chromium.org,mtrofin@chromium.org Review-Url: https://codereview.chromium.org/2795763002 Cr-Commit-Position: refs/heads/master@{#44388}
-