[v8-dev] Re: Add API support for passing a C++ function as a microtask callback (issue 306053003)
I apologize if I'm being dense, but from what I can tell storing a Foreign in |data| is completely safe. Clearly, storing a void* directly would not be. you're right. very sorry for the back and forth. SET_FIELD_WRAPPED was added recently i think and i didn't look into what it was doing. everything looks fine as is https://codereview.chromium.org/306053003/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from it, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] Re: First cut at run_mksnapshot action for gn (issue 293363009)
Committed patchset #2 manually as r21618 (presubmit successful). https://codereview.chromium.org/293363009/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from it, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] [v8] r21618 committed - First cut at run_mksnapshot action for gn...
Revision: 21618 Author: joc...@chromium.org Date: Tue Jun 3 06:50:46 2014 UTC Log: First cut at run_mksnapshot action for gn BUG=none R=bre...@chromium.org LOG=n Review URL: https://codereview.chromium.org/293363009 http://code.google.com/p/v8/source/detail?r=21618 Added: /branches/bleeding_edge/tools/run.py Modified: /branches/bleeding_edge/BUILD.gn === --- /dev/null +++ /branches/bleeding_edge/tools/run.pyTue Jun 3 06:50:46 2014 UTC @@ -0,0 +1,12 @@ +#!/usr/bin/env python +# Copyright 2014 the V8 project authors. All rights reserved. +# Use of this source code is governed by a BSD-style license that can be +# found in the LICENSE file. + +This program wraps an arbitrary command since gn currently can only execute +scripts. + +import subprocess +import sys + +sys.exit(subprocess.call(sys.argv[1:])) === --- /branches/bleeding_edge/BUILD.gnMon Jun 2 11:10:03 2014 UTC +++ /branches/bleeding_edge/BUILD.gnTue Jun 3 06:50:46 2014 UTC @@ -15,9 +15,10 @@ v8_object_print = false v8_postmortem_support = false v8_use_default_platform = true -#v8_use_snapshot = true +v8_use_snapshot = true v8_enable_extra_checks = is_debug v8_target_arch = cpu_arch +v8_random_seed = 314159265 ### @@ -185,6 +186,8 @@ src/regexp.js, src/arraybuffer.js, src/typedarray.js, +src/weak_collection.js, +src/promise.js, src/object-observe.js, src/macros.py, ] @@ -199,7 +202,7 @@ args = rebase_path(outputs, root_build_dir) + -[ EXPERIMENTAL, v8_compress_startup_data ] + +[ CORE, v8_compress_startup_data ] + rebase_path(sources, root_build_dir) } @@ -217,8 +220,6 @@ src/symbol.js, src/proxy.js, src/collection.js, -src/weak_collection.js, -src/promise.js, src/generator.js, src/array-iterator.js, src/harmony-string.js, @@ -232,7 +233,7 @@ args = rebase_path(outputs, root_build_dir) + -[ CORE, v8_compress_startup_data ] + +[ EXPERIMENTAL, v8_compress_startup_data ] + rebase_path(sources, root_build_dir) } @@ -255,6 +256,33 @@ rebase_path(sources, root_build_dir) } +action(run_mksnapshot) { + visibility = :* # Only targets in this file can depend on this. + + deps = [ :mksnapshot($host_toolchain) ] + + script = tools/run.py + + outputs = [ +$target_gen_dir/snapshot.cc + ] + + args = [ +./ + rebase_path(get_label_info(:mksnapshot($host_toolchain), + root_out_dir) + /mksnapshot, + root_build_dir), +--log-snapshot-positions, +--logfile, rebase_path($target_gen_dir/snapshot.log, root_build_dir) + ] + + if (v8_random_seed != 0) { +args += [ --random-seed, v8_random_seed ] + } + + args += rebase_path(outputs, root_build_dir) +} + + ### # Source Sets (aka static libraries) # @@ -280,6 +308,29 @@ configs += [ //build/config/compiler:no_chromium_code ] configs += [ :internal_config, :features, :toolchain ] } + +source_set(v8_snapshot) { + visibility = :* # Only targets in this file can depend on this. + + deps = [ +:js2c, +:js2c_experimental, +:generate_trig_table, +:run_mksnapshot, +:v8_base, + ] + + sources = [ +$target_gen_dir/libraries.cc, +$target_gen_dir/experimental-libraries.cc, +$target_gen_dir/trig-table.cc, +$target_gen_dir/snapshot.cc, + ] + + configs -= [ //build/config/compiler:chromium_code ] + configs += [ //build/config/compiler:no_chromium_code ] + configs += [ :internal_config, :features, :toolchain ] +} source_set(v8_base) { visibility = :* # Only targets in this file can depend on this. @@ -908,17 +959,10 @@ # Executables # -if (current_toolchain != host_toolchain is_android) { +executable(mksnapshot) { + visibility = :* # Only targets in this file can depend on this. -source_set(mksnapshot) { sources = [ - ] -} - -} else { - -executable(mksnapshot) { - sources = [ src/mksnapshot.cc, ] @@ -936,7 +980,55 @@ } } +### +# Public targets +# + +if (component_mode == shared_library) { + +component(v8) { + sources = [ +src/v8dll-main.cc, + ] + + if (v8_use_snapshot) { +deps = [ + :v8_base, + :v8_snapshot, +] + } else { +deps = [ + :v8_base, + :v8_nosnapshot, +] + } + + if (is_android current_toolchain != host_toolchain) { +libs = [ log ] + } + + configs -= [ //build/config/compiler:chromium_code ] + configs += [ //build/config/compiler:no_chromium_code ] + configs += [ :internal_config, :features, :toolchain ] + + # TODO(jochen): Support direct dependent configs. } +} else { + group(v8) { + if (v8_use_snapshot) { +deps = [ +
[v8-dev] Re: Trigonometric functions using fdlibm. (issue 303753002)
Reviewers: Raymond Toy, https://codereview.chromium.org/303753002/diff/40001/src/math.js File src/math.js (right): https://codereview.chromium.org/303753002/diff/40001/src/math.js#newcode262 src/math.js:262: } On 2014/06/02 17:26:11, Raymond Toy wrote: As you mentioned via email, you've removed the 3rd iteration. This is really needed if you want to be able to reduce multiples of pi/2 accurately. That's true. However, the reduction step is not exposed as a library function. From what I have seen, the third step seems to only affect y1. With a y0 really close to y1, it does not change the result of sine or cosine. This is also why I was asking for a test case where removing this third step would make a difference. Description: Trigonometric functions using fdlibm. BUG=v8:3006 LOG=N Please review this at https://codereview.chromium.org/303753002/ SVN Base: https://v8.googlecode.com/svn/branches/bleeding_edge Affected files (+655, -281 lines): M src/bootstrapper.cc M src/math.js A src/rempio2.h A src/rempio2.cc M src/runtime.h M src/runtime.cc D src/trig-table.h A + test/mjsunit/runtime-gen/rempio2.js M test/mjsunit/sin-cos.js M tools/generate-runtime-tests.py D tools/generate-trig-table.py M tools/gyp/v8.gyp -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from it, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] Re: When flag --nouse-osr is set, don't allow osr from hidden runtime calls. (issue 310773003)
LGTM, but the test case can be a *lot* simpler. Something like Flags: --nouse-osr function f() { %OptimizeFunctionOnNextCall(f, osr); for (var i = 0; i 1000; i++); } f(); https://codereview.chromium.org/310773003/diff/1/src/runtime.cc File src/runtime.cc (right): https://codereview.chromium.org/310773003/diff/1/src/runtime.cc#newcode8718 src/runtime.cc:8718: ASSERT(FLAG_use_osr); RUNTIME_ASSERT is more appropriate. https://codereview.chromium.org/310773003/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from it, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] Re: ARM64: Fix ASM_LOCATION and the like. (issue 308023007)
Thank you for checking it with cross builds. LGTM. https://codereview.chromium.org/308023007/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from it, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] %ObjectFreeze needs to exclude non-fast-path objects. (issue 315533002)
Reviewers: Yang, Message: Hi Yang, here is one more, thx! --Michael Description: %ObjectFreeze needs to exclude non-fast-path objects. ClusterFuzz will call it with sloppy arguments and similar cases. BUG=380049 LOG=N R=yang...@chromium.org Please review this at https://codereview.chromium.org/315533002/ SVN Base: https://v8.googlecode.com/svn/branches/bleeding_edge Affected files (+12, -7 lines): M src/runtime.cc A + test/mjsunit/regress/regress-380049.js Index: src/runtime.cc diff --git a/src/runtime.cc b/src/runtime.cc index 5971e33082b083d6292d2957dd5ef02b1e6fb04f..568efc1083566e2b517979fd5a4a4b5ead217e43 100644 --- a/src/runtime.cc +++ b/src/runtime.cc @@ -3258,6 +3258,12 @@ RUNTIME_FUNCTION(Runtime_ObjectFreeze) { HandleScope scope(isolate); ASSERT(args.length() == 1); CONVERT_ARG_HANDLE_CHECKED(JSObject, object, 0); + + // %ObjectFreeze is a fast path and these cases are handled elsewhere. + RUNTIME_ASSERT(!object-HasSloppyArgumentsElements() + !object-map()-is_observed() + !object-IsJSProxy()); + HandleObject result; ASSIGN_RETURN_FAILURE_ON_EXCEPTION(isolate, result, JSObject::Freeze(object)); return *result; Index: test/mjsunit/regress/regress-380049.js diff --git a/test/mjsunit/regress/regress-355486.js b/test/mjsunit/regress/regress-380049.js similarity index 64% copy from test/mjsunit/regress/regress-355486.js copy to test/mjsunit/regress/regress-380049.js index 55362a13416335b72bfa1ff92bc29f7a04edbd65..0b2b265fef27a8933bd825515fdeb814930751ef 100644 --- a/test/mjsunit/regress/regress-355486.js +++ b/test/mjsunit/regress/regress-380049.js @@ -4,10 +4,9 @@ // Flags: --allow-natives-syntax -function f() { var v = arguments[0]; } -function g() { f(); } - -g(); -g(); -%OptimizeFunctionOnNextCall(g); -g(); +function foo(a,b,c) { return arguments; } +var f = foo(false, null, 40); +try { + %ObjectFreeze(f); +} catch(e) { +} -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from it, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] Re: %ObjectFreeze needs to exclude non-fast-path objects. (issue 315533002)
lgtm. https://codereview.chromium.org/315533002/diff/1/test/mjsunit/regress/regress-380049.js File test/mjsunit/regress/regress-380049.js (right): https://codereview.chromium.org/315533002/diff/1/test/mjsunit/regress/regress-380049.js#newcode10 test/mjsunit/regress/regress-380049.js:10: %ObjectFreeze(f); how about assertThrows https://codereview.chromium.org/315533002/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from it, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] Add collection-iterator.js to BUILD.gn after r21615 (issue 314623002)
Reviewers: adamk, Description: Add collection-iterator.js to BUILD.gn after r21615 LOG=n BUG=none TBR=ad...@chromium.org Please review this at https://codereview.chromium.org/314623002/ SVN Base: https://v8.googlecode.com/svn/branches/bleeding_edge Affected files (+2, -1 lines): M BUILD.gn Index: BUILD.gn diff --git a/BUILD.gn b/BUILD.gn index 72ae64202d8e613cc65da175c706425fe4c8c618..5a73ec62fb0ba4310ed14570424f3f5b44f9c44c 100644 --- a/BUILD.gn +++ b/BUILD.gn @@ -220,6 +220,7 @@ action(js2c_experimental) { src/symbol.js, src/proxy.js, src/collection.js, +src/collection-iterator.js, src/generator.js, src/array-iterator.js, src/harmony-string.js, @@ -268,7 +269,7 @@ action(run_mksnapshot) { ] args = [ -./ + rebase_path(get_label_info(:mksnapshot($host_toolchain), +./ + rebase_path(get_label_info(:mksnapshot($host_toolchain), root_out_dir) + /mksnapshot, root_build_dir), --log-snapshot-positions, -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from it, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] Re: Remove forced type changes when they can't deopt (issue 303263010)
Looks good. Adding Jakob to reviewers. https://codereview.chromium.org/303263010/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from it, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] [v8] r21619 committed - Add collection-iterator.js to BUILD.gn after r21615...
Revision: 21619 Author: joc...@chromium.org Date: Tue Jun 3 07:26:07 2014 UTC Log: Add collection-iterator.js to BUILD.gn after r21615 LOG=n BUG=none TBR=ad...@chromium.org Review URL: https://codereview.chromium.org/314623002 http://code.google.com/p/v8/source/detail?r=21619 Modified: /branches/bleeding_edge/BUILD.gn === --- /branches/bleeding_edge/BUILD.gnTue Jun 3 06:50:46 2014 UTC +++ /branches/bleeding_edge/BUILD.gnTue Jun 3 07:26:07 2014 UTC @@ -220,6 +220,7 @@ src/symbol.js, src/proxy.js, src/collection.js, +src/collection-iterator.js, src/generator.js, src/array-iterator.js, src/harmony-string.js, @@ -268,7 +269,7 @@ ] args = [ -./ + rebase_path(get_label_info(:mksnapshot($host_toolchain), +./ + rebase_path(get_label_info(:mksnapshot($host_toolchain), root_out_dir) + /mksnapshot, root_build_dir), --log-snapshot-positions, -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from it, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] Re: Add collection-iterator.js to BUILD.gn after r21615 (issue 314623002)
Committed patchset #1 manually as r21619 (presubmit successful). https://codereview.chromium.org/314623002/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from it, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] Re: ARM64: Avoid regeneration of constants. (issue 308313002)
I am not sure why !info()-IsStub() check is triggering, but the approach with EmitAtUses seems much better. As Danno mentioned we would need a predicate for each architecture that decides if it is worthwhile to store constants in registers. https://codereview.chromium.org/308313002/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from it, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] Re: Tenure allocation sites only when semi-space is maximum size. (issue 309623007)
Done. https://codereview.chromium.org/309623007/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from it, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] Re: ARM64: Fix ASM_LOCATION and the like. (issue 308023007)
Committed patchset #2 manually as r21620 (presubmit successful). https://codereview.chromium.org/308023007/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from it, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] [v8] r21620 committed - ARM64: Fix ASM_LOCATION and the like....
Revision: 21620 Author: jacob.bram...@arm.com Date: Tue Jun 3 07:37:16 2014 UTC Log: ARM64: Fix ASM_LOCATION and the like. BUG= R=u...@chromium.org Review URL: https://codereview.chromium.org/308023007 http://code.google.com/p/v8/source/detail?r=21620 Modified: /branches/bleeding_edge/src/checks.h === --- /branches/bleeding_edge/src/checks.hWed May 28 08:07:18 2014 UTC +++ /branches/bleeding_edge/src/checks.hTue Jun 3 07:37:16 2014 UTC @@ -8,6 +8,7 @@ #include string.h #include ../include/v8stdint.h +#include base/build_config.h extern C void V8_Fatal(const char* file, int line, const char* format, ...); @@ -30,7 +31,8 @@ #endif // Simulator specific helpers. -#if defined(USE_SIMULATOR) defined(V8_TARGET_ARCH_ARM64) +// We can't use USE_SIMULATOR here because it isn't defined yet. +#if V8_TARGET_ARCH_ARM64 !V8_HOST_ARCH_ARM64 // TODO(all): If possible automatically prepend an indicator like // UNIMPLEMENTED or LOCATION. #define ASM_UNIMPLEMENTED(message) \ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from it, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] Re: Minor cleanups trivial refactoring related to Ast. (issue 298143004)
Committed patchset #2 manually as r21621 (presubmit successful). https://codereview.chromium.org/298143004/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from it, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] [v8] r21621 committed - Minor cleanups trivial refactoring related to Ast....
Revision: 21621 Author: ma...@chromium.org Date: Tue Jun 3 07:40:43 2014 UTC Log: Minor cleanups trivial refactoring related to Ast. 1) Literal::IsNull, IsTrue and IsFalse were dead code, and not needed. 2) No need to use the node type constants outside the Ast; there is IsSomeNodeType(). 3) AsSomeNodeType() != NULL - IsSomeNodeType(). R=rossb...@chromium.org BUG= Review URL: https://codereview.chromium.org/298143004 http://code.google.com/p/v8/source/detail?r=21621 Modified: /branches/bleeding_edge/src/arm/full-codegen-arm.cc /branches/bleeding_edge/src/arm64/full-codegen-arm64.cc /branches/bleeding_edge/src/ast.cc /branches/bleeding_edge/src/ast.h /branches/bleeding_edge/src/hydrogen.cc /branches/bleeding_edge/src/ia32/full-codegen-ia32.cc /branches/bleeding_edge/src/mips/full-codegen-mips.cc /branches/bleeding_edge/src/parser.cc /branches/bleeding_edge/src/x64/full-codegen-x64.cc /branches/bleeding_edge/src/x87/full-codegen-x87.cc === --- /branches/bleeding_edge/src/arm/full-codegen-arm.cc Wed May 28 16:00:52 2014 UTC +++ /branches/bleeding_edge/src/arm/full-codegen-arm.cc Tue Jun 3 07:40:43 2014 UTC @@ -2537,7 +2537,7 @@ // Assignment to a property, using a named store IC. Property* prop = expr-target()-AsProperty(); ASSERT(prop != NULL); - ASSERT(prop-key()-AsLiteral() != NULL); + ASSERT(prop-key()-IsLiteral()); // Record source code position before IC call. SetSourcePosition(expr-position()); === --- /branches/bleeding_edge/src/arm64/full-codegen-arm64.cc Wed May 28 16:00:52 2014 UTC +++ /branches/bleeding_edge/src/arm64/full-codegen-arm64.cc Tue Jun 3 07:40:43 2014 UTC @@ -2244,7 +2244,7 @@ // Assignment to a property, using a named store IC. Property* prop = expr-target()-AsProperty(); ASSERT(prop != NULL); - ASSERT(prop-key()-AsLiteral() != NULL); + ASSERT(prop-key()-IsLiteral()); // Record source code position before IC call. SetSourcePosition(expr-position()); === --- /branches/bleeding_edge/src/ast.cc Wed Apr 30 14:33:35 2014 UTC +++ /branches/bleeding_edge/src/ast.cc Tue Jun 3 07:40:43 2014 UTC @@ -34,17 +34,17 @@ bool Expression::IsSmiLiteral() const { - return AsLiteral() != NULL AsLiteral()-value()-IsSmi(); + return IsLiteral() AsLiteral()-value()-IsSmi(); } bool Expression::IsStringLiteral() const { - return AsLiteral() != NULL AsLiteral()-value()-IsString(); + return IsLiteral() AsLiteral()-value()-IsString(); } bool Expression::IsNullLiteral() const { - return AsLiteral() != NULL AsLiteral()-value()-IsNull(); + return IsLiteral() AsLiteral()-value()-IsNull(); } @@ -192,7 +192,7 @@ kind_ = PROTOTYPE; } else if (value_-AsMaterializedLiteral() != NULL) { kind_ = MATERIALIZED_LITERAL; - } else if (value_-AsLiteral() != NULL) { + } else if (value_-IsLiteral()) { kind_ = CONSTANT; } else { kind_ = COMPUTED; @@ -390,7 +390,7 @@ HandleObject MaterializedLiteral::GetBoilerplateValue(Expression* expression, Isolate* isolate) { - if (expression-AsLiteral() != NULL) { + if (expression-IsLiteral()) { return expression-AsLiteral()-value(); } if (CompileTimeValue::IsCompileTimeValue(expression)) { @@ -499,7 +499,7 @@ UnaryOperation* maybe_unary = expr-AsUnaryOperation(); return maybe_unary != NULL maybe_unary-op() == Token::VOID - maybe_unary-expression()-AsLiteral() != NULL; + maybe_unary-expression()-IsLiteral(); } === --- /branches/bleeding_edge/src/ast.h Mon May 26 13:59:24 2014 UTC +++ /branches/bleeding_edge/src/ast.h Tue Jun 3 07:40:43 2014 UTC @@ -1351,20 +1351,6 @@ virtual bool ToBooleanIsFalse() const V8_OVERRIDE { return !value_-BooleanValue(); } - - // Identity testers. - bool IsNull() const { -ASSERT(!value_.is_null()); -return value_-IsNull(); - } - bool IsTrue() const { -ASSERT(!value_.is_null()); -return value_-IsTrue(); - } - bool IsFalse() const { -ASSERT(!value_.is_null()); -return value_-IsFalse(); - } HandleObject value() const { return value_; } === --- /branches/bleeding_edge/src/hydrogen.cc Tue Jun 3 04:01:34 2014 UTC +++ /branches/bleeding_edge/src/hydrogen.cc Tue Jun 3 07:40:43 2014 UTC @@ -9149,7 +9149,7 @@ CHECK_ALIVE(VisitForValue(arguments-at(kObjectArg))); HValue* obj = Pop(); - if (arguments-at(kArrayIdArg)-node_type() != AstNode::kLiteral) { + if (arguments-at(kArrayIdArg)-IsLiteral()) { // This should never happen in real use, but can happen when fuzzing. // Just bail out. Bailout(kNeedSmiLiteral); @@ -9176,7 +9176,7 @@ HValue* byte_offset; bool is_zero_byte_offset; - if (arguments-at(kByteOffsetArg)-node_type() == AstNode::kLiteral + if
[v8-dev] Re: When flag --nouse-osr is set, don't allow osr from hidden runtime calls. (issue 310773003)
Thanks, I simplified the test and added the runtime assert. Much appreciated! --Michael https://codereview.chromium.org/310773003/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from it, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] [v8] r21622 committed - When flag --nouse-osr is set, don't allow osr from hidden runtime call...
Revision: 21622 Author: mvstan...@chromium.org Date: Tue Jun 3 07:45:40 2014 UTC Log: When flag --nouse-osr is set, don't allow osr from hidden runtime calls. BUG=379770 R=yang...@chromium.org LOG=N Review URL: https://codereview.chromium.org/310773003 http://code.google.com/p/v8/source/detail?r=21622 Added: /branches/bleeding_edge/test/mjsunit/regress/regress-379770.js Modified: /branches/bleeding_edge/src/runtime.cc === --- /dev/null +++ /branches/bleeding_edge/test/mjsunit/regress/regress-379770.js Tue Jun 3 07:45:40 2014 UTC @@ -0,0 +1,26 @@ +// Copyright 2014 the V8 project authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. +// Flags: --allow-natives-syntax --nostress-opt +// Flags: --nouse-osr + +function foo(obj) { + var counter = 1; + for (var i = 0; i obj.length; i++) { +%OptimizeFunctionOnNextCall(foo, osr); + } + counter += obj; + return counter; +} + +function bar() { + var a = [1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12]; + for (var i = 0; i 100; i++ ) { +foo(a); + } +} + +try { + bar(); +} catch (e) { +} === --- /branches/bleeding_edge/src/runtime.cc Tue Jun 3 00:53:16 2014 UTC +++ /branches/bleeding_edge/src/runtime.cc Tue Jun 3 07:45:40 2014 UTC @@ -8624,9 +8624,11 @@ // Start patching from the currently patched loop nesting level. int current_level = unoptimized-allow_osr_at_loop_nesting_level(); ASSERT(BackEdgeTable::Verify(isolate, unoptimized, current_level)); - for (int i = current_level + 1; i = Code::kMaxLoopNestingMarker; i++) { -unoptimized-set_allow_osr_at_loop_nesting_level(i); -isolate-runtime_profiler()-AttemptOnStackReplacement(*function); + if (FLAG_use_osr) { +for (int i = current_level + 1; i = Code::kMaxLoopNestingMarker; i++) { + unoptimized-set_allow_osr_at_loop_nesting_level(i); + isolate-runtime_profiler()-AttemptOnStackReplacement(*function); +} } } else if (type-IsOneByteEqualTo(STATIC_ASCII_VECTOR(concurrent)) isolate-concurrent_recompilation_enabled()) { @@ -8727,6 +8729,8 @@ // We're not prepared to handle a function with arguments object. ASSERT(!function-shared()-uses_arguments()); + RUNTIME_ASSERT(FLAG_use_osr); + // Passing the PC in the javascript frame from the caller directly is // not GC safe, so we walk the stack to get it. JavaScriptFrameIterator it(isolate); -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from it, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] Re: When flag --nouse-osr is set, don't allow osr from hidden runtime calls. (issue 310773003)
Committed patchset #2 manually as r21622 (presubmit successful). https://codereview.chromium.org/310773003/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from it, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] Re: Improve write barriers in optimized code. (issue 297763006)
https://codereview.chromium.org/297763006/diff/20001/src/arm/macro-assembler-arm.cc File src/arm/macro-assembler-arm.cc (right): https://codereview.chromium.org/297763006/diff/20001/src/arm/macro-assembler-arm.cc#newcode517 src/arm/macro-assembler-arm.cc:517: // TODO(mstarzinger): Dynamic counter missing. I just copied the TODO. https://codereview.chromium.org/297763006/diff/20001/src/ia32/macro-assembler-ia32.h File src/ia32/macro-assembler-ia32.h (right): https://codereview.chromium.org/297763006/diff/20001/src/ia32/macro-assembler-ia32.h#newcode144 src/ia32/macro-assembler-ia32.h:144: bool value_is_in_new_space = false); On 2014/06/03 05:51:48, Hannes Payer wrote: Can we make value_is_in_new_space an enum like SmiCheck e.g. PointersFromHereAreInterestingCheck? Done. https://codereview.chromium.org/297763006/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from it, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] [v8] r21623 committed - Tenure allocation sites only when semi-space is maximum size....
Revision: 21623 Author: hpa...@chromium.org Date: Tue Jun 3 07:55:38 2014 UTC Log: Tenure allocation sites only when semi-space is maximum size. BUG= R=mvstan...@chromium.org Review URL: https://codereview.chromium.org/309623007 http://code.google.com/p/v8/source/detail?r=21623 Modified: /branches/bleeding_edge/src/heap.cc /branches/bleeding_edge/src/heap.h /branches/bleeding_edge/src/mark-compact.cc /branches/bleeding_edge/src/objects-inl.h /branches/bleeding_edge/src/objects.cc /branches/bleeding_edge/src/objects.h /branches/bleeding_edge/src/spaces.h /branches/bleeding_edge/test/cctest/test-heap.cc === --- /branches/bleeding_edge/src/heap.cc Wed May 28 08:35:16 2014 UTC +++ /branches/bleeding_edge/src/heap.cc Tue Jun 3 07:55:38 2014 UTC @@ -101,6 +101,7 @@ promotion_rate_(0), semi_space_copied_object_size_(0), semi_space_copied_rate_(0), + maximum_size_scavenges_(0), max_gc_pause_(0.0), total_gc_time_ms_(0.0), max_alive_after_gc_(0), @@ -438,6 +439,13 @@ if (isolate()-concurrent_osr_enabled()) { isolate()-optimizing_compiler_thread()-AgeBufferedOsrJobs(); } + + if (new_space_.IsAtMaximumCapacity()) { +maximum_size_scavenges_++; + } else { +maximum_size_scavenges_ = 0; + } + CheckNewSpaceExpansionCriteria(); } @@ -485,12 +493,19 @@ // If the scratchpad overflowed, we have to iterate over the allocation // sites list. +// TODO(hpayer): We iterate over the whole list of allocation sites when +// we grew to the maximum semi-space size to deopt maybe tenured +// allocation sites. We could hold the maybe tenured allocation sites +// in a seperate data structure if this is a performance problem. bool use_scratchpad = -allocation_sites_scratchpad_length_ kAllocationSiteScratchpadSize; +allocation_sites_scratchpad_length_ kAllocationSiteScratchpadSize +new_space_.IsAtMaximumCapacity() +maximum_size_scavenges_ == 0; int i = 0; Object* list_element = allocation_sites_list(); bool trigger_deoptimization = false; +bool maximum_size_scavenge = MaximumSizeScavenge(); while (use_scratchpad ? i allocation_sites_scratchpad_length_ : list_element-IsAllocationSite()) { @@ -500,14 +515,17 @@ allocation_mementos_found += site-memento_found_count(); if (site-memento_found_count() 0) { active_allocation_sites++; - } - if (site-DigestPretenuringFeedback()) trigger_deoptimization = true; - if (site-GetPretenureMode() == TENURED) { -tenure_decisions++; - } else { -dont_tenure_decisions++; +if (site-DigestPretenuringFeedback(maximum_size_scavenge)) { + trigger_deoptimization = true; +} +if (site-GetPretenureMode() == TENURED) { + tenure_decisions++; +} else { + dont_tenure_decisions++; +} +allocation_sites++; } - allocation_sites++; + if (use_scratchpad) { i++; } else { @@ -1433,8 +1451,6 @@ // Used for updating survived_since_last_expansion_ at function end. intptr_t survived_watermark = PromotedSpaceSizeOfObjects(); - CheckNewSpaceExpansionCriteria(); - SelectScavengingVisitorsTable(); incremental_marking()-PrepareForScavenge(); === --- /branches/bleeding_edge/src/heap.h Tue May 27 13:43:29 2014 UTC +++ /branches/bleeding_edge/src/heap.h Tue Jun 3 07:55:38 2014 UTC @@ -1359,6 +1359,10 @@ void DeoptMarkedAllocationSites(); + bool MaximumSizeScavenge() { +return maximum_size_scavenges_ 0; + } + // ObjectStats are kept in two arrays, counts and sizes. Related stats are // stored in a contiguous linear buffer. Stats groups are stored one after // another. @@ -2019,6 +2023,12 @@ intptr_t semi_space_copied_object_size_; double semi_space_copied_rate_; + // This is the pretenuring trigger for allocation sites that are in maybe + // tenure state. When we switched to the maximum new space size we deoptimize + // the code that belongs to the allocation site and derive the lifetime + // of the allocation site. + unsigned int maximum_size_scavenges_; + // TODO(hpayer): Allocation site pretenuring may make this method obsolete. // Re-visit incremental marking heuristics. bool IsHighSurvivalRate() { === --- /branches/bleeding_edge/src/mark-compact.cc Mon Jun 2 11:41:50 2014 UTC +++ /branches/bleeding_edge/src/mark-compact.cc Tue Jun 3 07:55:38 2014 UTC @@ -3030,7 +3030,6 @@ // sweep collection by failing allocations. But since we are already in // a mark-sweep allocation, there is no sense in trying to trigger one. AlwaysAllocateScope scope(isolate()); - heap()-CheckNewSpaceExpansionCriteria(); NewSpace* new_space = heap()-new_space();
[v8-dev] Re: Tenure allocation sites only when semi-space is maximum size. (issue 309623007)
Committed patchset #4 manually as r21623 (presubmit successful). https://codereview.chromium.org/309623007/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from it, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] Re: Use full include paths everywhere (issue 304153016)
On 2014/06/02 17:24:56, tfarina wrote: What was the technical reason for doing this? Could you write it in the CL description? Is this something skia could be doing in future? dunno about skia... updated the description https://codereview.chromium.org/304153016/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from it, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] Re: %ObjectFreeze needs to exclude non-fast-path objects. (issue 315533002)
Committed patchset #2 manually as r21624 (presubmit successful). https://codereview.chromium.org/315533002/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from it, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] Re: ARM64: Avoid regeneration of constants. (issue 308313002)
Actually, the right thing to do is to treat constants as normal values and teach the register allocator how to split constant ranges and rematerialize on demand without spilling. This is something that Jaro and Benedikt have thought about recently, and I think this is generally a cleaner way to approach the problem than the ad-hoc EmitAsUses approach which is just a brittle heuristic. https://codereview.chromium.org/308313002/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from it, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] Fix PathTracer. (issue 316533002)
Reviewers: Yang, Message: PTAL Description: Fix PathTracer. When tracing, we abuse the map for marking, thereby mutating it. HeapObject::map() takes care of recovering unabused value. Please review this at https://codereview.chromium.org/316533002/ SVN Base: https://v8.googlecode.com/svn/branches/bleeding_edge Affected files (+28, -32 lines): M src/heap.h M src/heap.cc M src/objects-inl.h M test/cctest/test-heap.cc Index: src/heap.cc diff --git a/src/heap.cc b/src/heap.cc index ff7a9ff12f20d6f7e37751f7d2bd91fb26939684..28bd81b3c4a8dd33c5bfcc06d1133d801d1812e5 100644 --- a/src/heap.cc +++ b/src/heap.cc @@ -5837,9 +5837,8 @@ void PathTracer::MarkRecursively(Object** p, MarkVisitor* mark_visitor) { HeapObject* obj = HeapObject::cast(*p); - Object* map = obj-map(); - - if (!map-IsHeapObject()) return; // visited before + MapWord map_word = obj-map_word(); + if (!map_word.ToMap()-IsHeapObject()) return; // visited before if (found_target_in_trace_) return; // stop if target found object_stack_.Add(obj); @@ -5853,11 +5852,11 @@ void PathTracer::MarkRecursively(Object** p, MarkVisitor* mark_visitor) { bool is_native_context = SafeIsNativeContext(obj); // not visited yet - Map* map_p = reinterpret_castMap*(HeapObject::cast(map)); - - Address map_addr = map_p-address(); + Map* map = Map::cast(map_word.ToMap()); - obj-set_map_no_write_barrier(reinterpret_castMap*(map_addr + kMarkTag)); + MapWord marked_map_word = + MapWord::FromRawValue(obj-map_word().ToRawValue() + kMarkTag); + obj-set_map_word(marked_map_word); // Scan the object body. if (is_native_context (visit_mode_ == VISIT_ONLY_STRONG)) { @@ -5868,17 +5867,16 @@ void PathTracer::MarkRecursively(Object** p, MarkVisitor* mark_visitor) { Context::kHeaderSize + Context::FIRST_WEAK_SLOT * kPointerSize); mark_visitor-VisitPointers(start, end); } else { -obj-IterateBody(map_p-instance_type(), - obj-SizeFromMap(map_p), - mark_visitor); +obj-IterateBody(map-instance_type(), obj-SizeFromMap(map), mark_visitor); } // Scan the map after the body because the body is a lot more interesting // when doing leak detection. - MarkRecursively(map, mark_visitor); + MarkRecursively(reinterpret_castObject**(map), mark_visitor); - if (!found_target_in_trace_) // don't pop if found the target + if (!found_target_in_trace_) { // don't pop if found the target object_stack_.RemoveLast(); + } } @@ -5887,25 +5885,18 @@ void PathTracer::UnmarkRecursively(Object** p, UnmarkVisitor* unmark_visitor) { HeapObject* obj = HeapObject::cast(*p); - Object* map = obj-map(); - - if (map-IsHeapObject()) return; // unmarked already - - Address map_addr = reinterpret_castAddress(map); - - map_addr -= kMarkTag; - - ASSERT_TAG_ALIGNED(map_addr); + MapWord map_word = obj-map_word(); + if (map_word.ToMap()-IsHeapObject()) return; // unmarked already - HeapObject* map_p = HeapObject::FromAddress(map_addr); + MapWord unmarked_map_word = + MapWord::FromRawValue(map_word.ToRawValue() - kMarkTag); + obj-set_map_word(unmarked_map_word); - obj-set_map_no_write_barrier(reinterpret_castMap*(map_p)); + Map* map = Map::cast(unmarked_map_word.ToMap()); - UnmarkRecursively(reinterpret_castObject**(map_p), unmark_visitor); + UnmarkRecursively(reinterpret_castObject**(map), unmark_visitor); - obj-IterateBody(Map::cast(map_p)-instance_type(), - obj-SizeFromMap(Map::cast(map_p)), - unmark_visitor); + obj-IterateBody(map-instance_type(), obj-SizeFromMap(map), unmark_visitor); } Index: src/heap.h diff --git a/src/heap.h b/src/heap.h index 8956e3c56b50f31af04c57a3caf9669d4c58f3f2..c7a7c82aa65c17252f376a681f9b318f40078493 100644 --- a/src/heap.h +++ b/src/heap.h @@ -2717,6 +2717,9 @@ class PathTracer : public ObjectVisitor { FIND_FIRST // Will stop the search after first match. }; + // Tags 0, 1, and 3 are used. Use 2 for marking visited HeapObject. + static const int kMarkTag = 2; + // For the WhatToFind arg, if FIND_FIRST is specified, tracing will stop // after the first match. If FIND_ALL is specified, then tracing will be // done for all matches. @@ -2748,9 +2751,6 @@ class PathTracer : public ObjectVisitor { void UnmarkRecursively(Object** p, UnmarkVisitor* unmark_visitor); virtual void ProcessResults(); - // Tags 0, 1, and 3 are used. Use 2 for marking visited HeapObject. - static const int kMarkTag = 2; - Object* search_target_; bool found_target_; bool found_target_in_trace_; Index: src/objects-inl.h diff --git a/src/objects-inl.h b/src/objects-inl.h index 42c46e0568f10f8b3289e3d555b02106c2234c56..7af165bd7efb0ec3e4cb6b93c12113ca784464ab 100644 --- a/src/objects-inl.h +++ b/src/objects-inl.h @@ -1328,7 +1328,14 @@ Isolate* HeapObject::GetIsolate() { Map* HeapObject::map() { +#ifdef DEBUG + // Clear mark potentially
[v8-dev] Re: Issue 3345 in v8: i18n: Make break iterator use optional
Updates: Cc: c...@chromium.org js...@chromium.org Comment #3 on issue 3345 by joc...@chromium.org: i18n: Make break iterator use optional http://code.google.com/p/v8/issues/detail?id=3345 cira@, can you comment on this? -- You received this message because this project is configured to send all issue notifications to this address. You may adjust your notification preferences at: https://code.google.com/hosting/settings -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from it, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] Re: Issue 3348 in v8: v8 Intl doesn't handle failure conditions
Updates: Cc: c...@chromium.org js...@chromium.org Comment #2 on issue 3348 by joc...@chromium.org: v8 Intl doesn't handle failure conditions http://code.google.com/p/v8/issues/detail?id=3348 I'm somewhat ambivalent about this one. I'd rather crash if the system wasn't configured correctly. -- You received this message because this project is configured to send all issue notifications to this address. You may adjust your notification preferences at: https://code.google.com/hosting/settings -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from it, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] Re: Issue 3344 in v8: Add v8::V8::InitializeICUDirectory(const char *path)
Updates: Cc: joc...@chromium.org Comment #1 on issue 3344 by joc...@chromium.org: Add v8::V8::InitializeICUDirectory(const char *path) http://code.google.com/p/v8/issues/detail?id=3344 The InitializeICU call in V8 was added only for testing (otherwise, e.g., you'd want to mmap the data files). I'm not sure whether it's worthwhile to add complexity here, since an embedder can just initialize ICU itself and not call V8::InitializeICU at all, wdyt? -- You received this message because this project is configured to send all issue notifications to this address. You may adjust your notification preferences at: https://code.google.com/hosting/settings -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from it, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] Re: Fix leak in debug mirror cache. (issue 296953005)
https://codereview.chromium.org/296953005/diff/40001/src/debug-debugger.js File src/debug-debugger.js (right): https://codereview.chromium.org/296953005/diff/40001/src/debug-debugger.js#newcode491 src/debug-debugger.js:491: return %DebugGetLoadedScripts(); This change looks wrong to me. By design of V8 debugging protocol mirror cache should be kept while we are staying on a breakpoint so that the client could access corresponding object by its mirror id. After this change, however, the cache will be cleared on any call to GetLoadedScripts() and the remote id will become invalid. In case of blink the problem is that we create mirror objects not only when we stay on a breakpoint and I'm not sure we clear them properly. Also we don't use id-mirror map in blink and I believe a right way to fix this would be to disable mirror cache in blink completely. WDYT? https://codereview.chromium.org/296953005/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from it, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] Re: Issue 3348 in v8: v8 Intl doesn't handle failure conditions
Updates: Cc: joc...@chromium.org Comment #3 on issue 3348 by joc...@chromium.org: v8 Intl doesn't handle failure conditions http://code.google.com/p/v8/issues/detail?id=3348 (No comment was entered for this change.) -- You received this message because this project is configured to send all issue notifications to this address. You may adjust your notification preferences at: https://code.google.com/hosting/settings -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from it, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] Re: Remove forced type changes when they can't deopt (issue 303263010)
On 2014/06/02 14:33:06, m.m.capewell wrote: the representation is unknown at the point that the forced change is introduced. That's exactly why putting this logic into the graph builder is the wrong approach. Any operation that needs to check the representation of HValues should be performed after the infer representations phase. Usually I'd suggest to make this a part of canonicalization, but you'll probably get better results by putting it directly into the insert representation changes phase. https://codereview.chromium.org/303263010/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from it, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] Re: Issue 3345 in v8: i18n: Make break iterator use optional
Updates: Cc: joc...@chromium.org Comment #4 on issue 3345 by joc...@chromium.org: i18n: Make break iterator use optional http://code.google.com/p/v8/issues/detail?id=3345 (No comment was entered for this change.) -- You received this message because this project is configured to send all issue notifications to this address. You may adjust your notification preferences at: https://code.google.com/hosting/settings -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from it, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] Re: Some v8 changes to support i18n
+v8-users, v8-dev is mostly for codereviews and stuff... On Tuesday, June 3, 2014 12:45:44 AM UTC+2, Steven R. Loomis (IBM) wrote: Hello, I would like to contribute some changes to support i18n use. I work on ICU at IBM, and it's my understanding that there is a CCLA sent in already. Yes, indeed! For background, I am working on trying to get ICU on by default in Node. https://github.com/joyent/node/pull/7719 Cool! I've opened three bugs: Thank you, I cc'd some folks and added comments to them * https://code.google.com/p/v8/issues/detail?id=3345 - Make breakiterator optional. This detects whether break iteration is turned off in the ICU you are built against. Saves ~1MB of code (not counting data). * https://code.google.com/p/v8/issues/detail?id=3348 - bug fixes This does some error checking if ICU wasn't able to load its data. This isn't a complete fix, but turns crashers (SEGV) into soft errors (node exits with an error message). And the more complicated one.. * https://code.google.com/p/v8/issues/detail?id=3344 - InitializeICUDirectory This just sets u_setDataDirectory() to tell ICU where to load its data, or if the param is null, it sets a small built in data pack, just en+root and calls u_setCommonData().. An alternate model could be to just have two new APIs: InitializeICUDirectory( path .. ) InitializeICUData( void *data .. ) The existing function, InitializeICU( const char *file ) could be kept, no conflict there. Just document to call only one of the functions. * As to building ICU itself, I have some new gyp+bash code, which might be better gyp+python, but have not disturbed at all how v8 builds against chromium's ICU. We're directly using chromium's ICU, however, it's possible to also use the system's ICU library or your own version. best -jochen Please let me know how best to proceed. Regards, Steven -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from it, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] Re: Fix leak in debug mirror cache. (issue 296953005)
https://codereview.chromium.org/296953005/diff/40001/src/debug-debugger.js File src/debug-debugger.js (right): https://codereview.chromium.org/296953005/diff/40001/src/debug-debugger.js#newcode491 src/debug-debugger.js:491: return %DebugGetLoadedScripts(); On 2014/06/03 08:07:21, yurys wrote: This change looks wrong to me. By design of V8 debugging protocol mirror cache should be kept while we are staying on a breakpoint so that the client could access corresponding object by its mirror id. After this change, however, the cache will be cleared on any call to GetLoadedScripts() and the remote id will become invalid. In case of blink the problem is that we create mirror objects not only when we stay on a breakpoint and I'm not sure we clear them properly. Also we don't use id-mirror map in blink and I believe a right way to fix this would be to disable mirror cache in blink completely. WDYT? Not sure I understand. Which code path does blink use to get loaded scripts? https://codereview.chromium.org/296953005/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from it, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] Re: Fix PathTracer. (issue 316533002)
On 2014/06/03 08:01:31, Igor Sheludko wrote: PTAL lgtm. https://codereview.chromium.org/316533002/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from it, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] Re: Use full include paths everywhere (issue 304153016)
Committed patchset #1 manually as r21625 (presubmit successful). https://codereview.chromium.org/304153016/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from it, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] Re: ARM64: Avoid regeneration of constants. (issue 308313002)
On 2014/06/03 08:01:16, danno wrote: Actually, the right thing to do is to treat constants as normal values and teach the register allocator how to split constant ranges and rematerialize on demand without spilling. This is something that Jaro and Benedikt have thought about recently, and I think this is generally a cleaner way to approach the problem than the ad-hoc EmitAsUses approach which is just a brittle heuristic. I did a bit of work on this as a follow-up for this patch: I augmented lithium instructions with a method indicating if it was better to regenerate the value (re-visit the instruction) instead of spilling and restoring it. LiveRanges and LOperands kept track of which lithium instruction output they referred to. The gap resolver skips spilling moves when appropriate, and instead of moving from the stack re-visits the lithium instruction. I think there is some more work to do around that, but if you are interested I could upload this so you can have a look. https://codereview.chromium.org/308313002/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from it, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] [v8] r21626 committed - Fix PathTracer....
Revision: 21626 Author: ish...@chromium.org Date: Tue Jun 3 08:28:38 2014 UTC Log: Fix PathTracer. When tracing, we abuse the map for marking, thereby mutating it. HeapObject::map() takes care of recovering unabused value. R=yang...@chromium.org Review URL: https://codereview.chromium.org/316533002 http://code.google.com/p/v8/source/detail?r=21626 Modified: /branches/bleeding_edge/src/heap.cc /branches/bleeding_edge/src/heap.h /branches/bleeding_edge/src/objects-inl.h /branches/bleeding_edge/test/cctest/test-heap.cc === --- /branches/bleeding_edge/src/heap.cc Tue Jun 3 08:12:43 2014 UTC +++ /branches/bleeding_edge/src/heap.cc Tue Jun 3 08:28:38 2014 UTC @@ -5853,9 +5853,8 @@ HeapObject* obj = HeapObject::cast(*p); - Object* map = obj-map(); - - if (!map-IsHeapObject()) return; // visited before + MapWord map_word = obj-map_word(); + if (!map_word.ToMap()-IsHeapObject()) return; // visited before if (found_target_in_trace_) return; // stop if target found object_stack_.Add(obj); @@ -5869,11 +5868,11 @@ bool is_native_context = SafeIsNativeContext(obj); // not visited yet - Map* map_p = reinterpret_castMap*(HeapObject::cast(map)); + Map* map = Map::cast(map_word.ToMap()); - Address map_addr = map_p-address(); - - obj-set_map_no_write_barrier(reinterpret_castMap*(map_addr + kMarkTag)); + MapWord marked_map_word = + MapWord::FromRawValue(obj-map_word().ToRawValue() + kMarkTag); + obj-set_map_word(marked_map_word); // Scan the object body. if (is_native_context (visit_mode_ == VISIT_ONLY_STRONG)) { @@ -5884,17 +5883,16 @@ Context::kHeaderSize + Context::FIRST_WEAK_SLOT * kPointerSize); mark_visitor-VisitPointers(start, end); } else { -obj-IterateBody(map_p-instance_type(), - obj-SizeFromMap(map_p), - mark_visitor); +obj-IterateBody(map-instance_type(), obj-SizeFromMap(map), mark_visitor); } // Scan the map after the body because the body is a lot more interesting // when doing leak detection. - MarkRecursively(map, mark_visitor); + MarkRecursively(reinterpret_castObject**(map), mark_visitor); - if (!found_target_in_trace_) // don't pop if found the target + if (!found_target_in_trace_) { // don't pop if found the target object_stack_.RemoveLast(); + } } @@ -5903,25 +5901,18 @@ HeapObject* obj = HeapObject::cast(*p); - Object* map = obj-map(); - - if (map-IsHeapObject()) return; // unmarked already - - Address map_addr = reinterpret_castAddress(map); - - map_addr -= kMarkTag; - - ASSERT_TAG_ALIGNED(map_addr); + MapWord map_word = obj-map_word(); + if (map_word.ToMap()-IsHeapObject()) return; // unmarked already - HeapObject* map_p = HeapObject::FromAddress(map_addr); + MapWord unmarked_map_word = + MapWord::FromRawValue(map_word.ToRawValue() - kMarkTag); + obj-set_map_word(unmarked_map_word); - obj-set_map_no_write_barrier(reinterpret_castMap*(map_p)); + Map* map = Map::cast(unmarked_map_word.ToMap()); - UnmarkRecursively(reinterpret_castObject**(map_p), unmark_visitor); + UnmarkRecursively(reinterpret_castObject**(map), unmark_visitor); - obj-IterateBody(Map::cast(map_p)-instance_type(), - obj-SizeFromMap(Map::cast(map_p)), - unmark_visitor); + obj-IterateBody(map-instance_type(), obj-SizeFromMap(map), unmark_visitor); } === --- /branches/bleeding_edge/src/heap.h Tue Jun 3 08:12:43 2014 UTC +++ /branches/bleeding_edge/src/heap.h Tue Jun 3 08:28:38 2014 UTC @@ -2727,6 +2727,9 @@ FIND_FIRST // Will stop the search after first match. }; + // Tags 0, 1, and 3 are used. Use 2 for marking visited HeapObject. + static const int kMarkTag = 2; + // For the WhatToFind arg, if FIND_FIRST is specified, tracing will stop // after the first match. If FIND_ALL is specified, then tracing will be // done for all matches. @@ -2758,9 +2761,6 @@ void UnmarkRecursively(Object** p, UnmarkVisitor* unmark_visitor); virtual void ProcessResults(); - // Tags 0, 1, and 3 are used. Use 2 for marking visited HeapObject. - static const int kMarkTag = 2; - Object* search_target_; bool found_target_; bool found_target_in_trace_; === --- /branches/bleeding_edge/src/objects-inl.h Tue Jun 3 08:12:43 2014 UTC +++ /branches/bleeding_edge/src/objects-inl.h Tue Jun 3 08:28:38 2014 UTC @@ -1328,7 +1328,14 @@ Map* HeapObject::map() { +#ifdef DEBUG + // Clear mark potentially added by PathTracer. + uintptr_t raw_value = + map_word().ToRawValue() ~static_castuintptr_t(PathTracer::kMarkTag); + return MapWord::FromRawValue(raw_value).ToMap(); +#else return map_word().ToMap(); +#endif } === --- /branches/bleeding_edge/test/cctest/test-heap.cc Tue Jun 3 08:12:43 2014 UTC +++
[v8-dev] Re: Fix PathTracer. (issue 316533002)
Committed patchset #1 manually as r21626 (presubmit successful). https://codereview.chromium.org/316533002/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from it, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] Fix compilation on win shared and mips (issue 309983002)
Reviewers: mvstanton, Description: Fix compilation on win shared and mips TBR=mvstan...@chromium.org LOG=n BUG=none Please review this at https://codereview.chromium.org/309983002/ SVN Base: https://v8.googlecode.com/svn/branches/bleeding_edge Affected files (+5, -2 lines): M src/mips/frames-mips.cc M test/cctest/cctest.cc M tools/gyp/v8.gyp Index: src/mips/frames-mips.cc diff --git a/src/mips/frames-mips.cc b/src/mips/frames-mips.cc index 55f98cfa98b9fa06df20a504d1222f69a8318704..5da00801c775cc27f2a6aa6e003003422bd19dc2 100644 --- a/src/mips/frames-mips.cc +++ b/src/mips/frames-mips.cc @@ -8,7 +8,7 @@ #include src/assembler.h #include src/mips/assembler-mips.h -#include src/amips/ssembler-mips-inl.h +#include src/mips/assembler-mips-inl.h #include src/frames.h namespace v8 { Index: test/cctest/cctest.cc diff --git a/test/cctest/cctest.cc b/test/cctest/cctest.cc index abb8961060d9001fdb7e8630f29a2253b6cf5abc..0be152064e7fd28b0e06356fc98e3e99a2fc5aee 100644 --- a/test/cctest/cctest.cc +++ b/test/cctest/cctest.cc @@ -25,7 +25,7 @@ // (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE // OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. -#include v8.h +#include include/v8.h #include test/cctest/cctest.h #include src/debug.h Index: tools/gyp/v8.gyp diff --git a/tools/gyp/v8.gyp b/tools/gyp/v8.gyp index e499eb553f391db3f825baa4795e05c2c8fcf4ee..694677db4ea94cd564d3ed4a9f696b96898c2f24 100644 --- a/tools/gyp/v8.gyp +++ b/tools/gyp/v8.gyp @@ -61,6 +61,9 @@ # has some sources to link into the component. '../../src/v8dll-main.cc', ], + 'include_dirs': [ +'../..', + ], 'defines': [ 'V8_SHARED', 'BUILDING_V8_SHARED', -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from it, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] Re: Fix compilation on win shared and mips (issue 309983002)
Committed patchset #1 manually as r21627 (presubmit successful). https://codereview.chromium.org/309983002/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from it, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] [v8] r21627 committed - Fix compilation on win shared and mips...
Revision: 21627 Author: joc...@chromium.org Date: Tue Jun 3 08:29:03 2014 UTC Log: Fix compilation on win shared and mips TBR=mvstan...@chromium.org LOG=n BUG=none Review URL: https://codereview.chromium.org/309983002 http://code.google.com/p/v8/source/detail?r=21627 Modified: /branches/bleeding_edge/src/mips/frames-mips.cc /branches/bleeding_edge/test/cctest/cctest.cc /branches/bleeding_edge/tools/gyp/v8.gyp === --- /branches/bleeding_edge/src/mips/frames-mips.cc Tue Jun 3 08:12:43 2014 UTC +++ /branches/bleeding_edge/src/mips/frames-mips.cc Tue Jun 3 08:29:03 2014 UTC @@ -8,7 +8,7 @@ #include src/assembler.h #include src/mips/assembler-mips.h -#include src/amips/ssembler-mips-inl.h +#include src/mips/assembler-mips-inl.h #include src/frames.h namespace v8 { === --- /branches/bleeding_edge/test/cctest/cctest.cc Tue Jun 3 08:12:43 2014 UTC +++ /branches/bleeding_edge/test/cctest/cctest.cc Tue Jun 3 08:29:03 2014 UTC @@ -25,7 +25,7 @@ // (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE // OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. -#include v8.h +#include include/v8.h #include test/cctest/cctest.h #include src/debug.h === --- /branches/bleeding_edge/tools/gyp/v8.gypTue Jun 3 08:12:43 2014 UTC +++ /branches/bleeding_edge/tools/gyp/v8.gypTue Jun 3 08:29:03 2014 UTC @@ -60,6 +60,9 @@ # Note: on non-Windows we still build this file so that gyp # has some sources to link into the component. '../../src/v8dll-main.cc', + ], + 'include_dirs': [ +'../..', ], 'defines': [ 'V8_SHARED', -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from it, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] Remove duplicate code in SetPropertyPostInterceptor (issue 314673002)
Reviewers: Benedikt Meurer, Message: PTAL Description: Remove duplicate code in SetPropertyPostInterceptor BUG= Please review this at https://codereview.chromium.org/314673002/ SVN Base: https://v8.googlecode.com/svn/branches/bleeding_edge Affected files (+2, -16 lines): M src/objects.cc Index: src/objects.cc diff --git a/src/objects.cc b/src/objects.cc index 1a63712ee9d9e1c673f77e13447e9f69214571fb..82d26be01426969293366e7bcf73fc022e7ef059 100644 --- a/src/objects.cc +++ b/src/objects.cc @@ -2081,22 +2081,8 @@ MaybeHandleObject JSObject::SetPropertyPostInterceptor( if (!result.IsFound()) { object-map()-LookupTransition(*object, *name, result); } - if (result.IsFound()) { -// An existing property or a map transition was found. Use set property to -// handle all these cases. -return SetPropertyForResult(object, result, name, value, attributes, -strict_mode, MAY_BE_STORE_FROM_KEYED); - } - bool done = false; - HandleObject result_object; - ASSIGN_RETURN_ON_EXCEPTION( - isolate, result_object, - SetPropertyViaPrototypes( - object, name, value, attributes, strict_mode, done), - Object); - if (done) return result_object; - // Add a new real property. - return AddProperty(object, name, value, attributes, strict_mode); + return SetPropertyForResult(object, result, name, value, attributes, + strict_mode, MAY_BE_STORE_FROM_KEYED); } -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from it, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] Re: Remove duplicate code in SetPropertyPostInterceptor (issue 314673002)
lgtm https://codereview.chromium.org/314673002/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from it, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
Re: [v8-dev] Garbage collection self-modifying code
On 2 June 2014 20:55, Hendrik Greving hendrik.greving@gmail.com wrote: Whenever V8 has to flush the ICache on ARM, this is due to self-modifying code. I understand this e.g. happens when we patch return points to optimized code with calls to de-optimization runtime functions. What I haven't understood is if and when garbage collection actually modifies code objects (not moving them, if code is moved, that's not SMC, and shouldn't require to flush the ICache, It does! On ARM you need to manually ensure coherency of the I and D cache and when the code is moved it is moved as normal data and the I cache may not see the move. The same apply when you load a program in memory. SMC is just one case when the caches need syncing. So in which cases is code actually getting patched (by garbage collection), e.g. is code getting patched with new pointer literal values? That can happen too. Then the second part I'd wanted to understand was where in V8 do we make sure that when e.g. pointer values are changed (because we moved data on the heap), that the hardware state in the processor is updated as well (some pointers might have lived in registers)? When a GC occurs everything has been written to memory and will be updated. Among other things look for Safepoints. -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from it, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] Folding of bounded dynamic size allocations with const size allocations. (issue 301973014)
Reviewers: Hannes Payer, Message: PTAL Description: Folding of bounded dynamic size allocations with const size allocations. Please review this at https://codereview.chromium.org/301973014/ SVN Base: https://v8.googlecode.com/svn/branches/bleeding_edge Affected files (+86, -59 lines): M src/hydrogen-instructions.h M src/hydrogen-instructions.cc -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from it, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] Re: WIP: Parser: delay string internalization. (issue 231073002)
(Marking comments done here. I'll create a new CL as this is moving away from the WIP state...) https://codereview.chromium.org/231073002/diff/880002/src/ast.h File src/ast.h (right): https://codereview.chromium.org/231073002/diff/880002/src/ast.h#newcode852 src/ast.h:852: ForStatement(Zone* zone, ZoneListParserSymbolTable::Symbol** labels, int pos) On 2014/05/08 12:52:08, rossberg wrote: Nit: line length Done. https://codereview.chromium.org/231073002/diff/880002/src/ast.h#newcode894 src/ast.h:894: ForEachStatement(Zone* zone, ZoneListParserSymbolTable::Symbol** labels, int pos) On 2014/05/08 12:52:08, rossberg wrote: Nit: line length Done. https://codereview.chromium.org/231073002/diff/880002/src/ast.h#newcode934 src/ast.h:934: ForInStatement(Zone* zone, ZoneListParserSymbolTable::Symbol** labels, int pos) On 2014/05/08 12:52:08, rossberg wrote: Nit: line length Done. https://codereview.chromium.org/231073002/diff/880002/src/ast.h#newcode997 src/ast.h:997: ForOfStatement(Zone* zone, ZoneListParserSymbolTable::Symbol** labels, int pos) On 2014/05/08 12:52:08, rossberg wrote: Nit: line length Done. https://codereview.chromium.org/231073002/diff/880002/src/ast.h#newcode1157 src/ast.h:1157: SwitchStatement(Zone* zone, ZoneListParserSymbolTable::Symbol** labels, int pos) On 2014/05/08 12:52:08, rossberg wrote: Nit: line length Done. https://codereview.chromium.org/231073002/diff/880002/src/ast.h#newcode1382 src/ast.h:1382: if (!value_.is_null()) { On 2014/05/08 12:52:08, rossberg wrote: Is it an invariant that at least one of value_, string_, strings_ is non-null? If so, might be worth asserting it here. Or alternatively, add an else UNREACHABLE below. Likewise, one could assert that at most one of string_ and strings_ can't be non-null. This is now refactored so that we have another Literal class which takes care of this indirection (literal can be bool / double / int / string / array of strings) - obsolete. https://codereview.chromium.org/231073002/diff/880002/src/ast.h#newcode1402 src/ast.h:1402: ParserSymbolTable::Symbol* string() const { return string_; } On 2014/05/08 12:52:08, rossberg wrote: Maybe call these symbol/symbol_? This is also in the other class now, and I think it's reasonable to call it string there (as in string / boolean / number). https://codereview.chromium.org/231073002/diff/880002/src/ast.h#newcode1442 src/ast.h:1442: ZoneListParserSymbolTable::Symbol** strings_; On 2014/05/08 12:52:08, rossberg wrote: symbols_? Also, add a comment that this is to represent arrays of strings. Ditto (added comment about this to the literal class) https://codereview.chromium.org/231073002/diff/880002/src/ast.h#newcode2383 src/ast.h:2383: if (raw_inferred_name_ != NULL) { On 2014/05/08 12:52:08, rossberg wrote: I'm surprised that inferred_name_ does not take precedence. Doesn't matter, because only one of them will be set. Made that explicit in the code with ASSERTs and nullifying the raw_inferred_name_ if name_ is set, and vice versa. https://codereview.chromium.org/231073002/diff/880002/src/parser-symbol-table.h File src/parser-symbol-table.h (right): https://codereview.chromium.org/231073002/diff/880002/src/parser-symbol-table.h#newcode66 src/parser-symbol-table.h:66: class ParserSymbolTable { On 2014/05/08 12:52:08, rossberg wrote: How about naming this just SymbolTable? It's not quite specific to the parser, as symbols are used after parsing, and in modules that don't (or shouldn't) know anything about parsing (e.g., scopes, interfaces). Also, I feel tempted to suggest using a different term than symbol, in order to avoid confusion with ES6 symbols. One common synonym is atom. Done (AstStringTable etc. as discussed offline). https://codereview.chromium.org/231073002/diff/880002/src/parser-symbol-table.h#newcode78 src/parser-symbol-table.h:78: void AlwaysInternalize(Isolate* isolate) { On 2014/05/08 12:52:08, rossberg wrote: How does this differ from Internalize (which also sets isolate_)? - Removed AlwaysInternalize https://codereview.chromium.org/231073002/diff/880002/src/parser-symbol-table.h#newcode149 src/parser-symbol-table.h:149: // FIXME: these are easily created with a macro. On 2014/05/08 12:52:08, rossberg wrote: I was about to suggest that. :) Done. https://codereview.chromium.org/231073002/diff/880002/src/parser.cc File src/parser.cc (right): https://codereview.chromium.org/231073002/diff/880002/src/parser.cc#newcode1031 src/parser.cc:1031: // Start using the heap (before scope goes out of scope). On 2014/05/08 12:52:08, rossberg wrote: scope is exited? Actually, this is not needed any more, so moved the internalizing later. https://codereview.chromium.org/231073002/diff/880002/src/parser.cc#newcode1093 src/parser.cc:1093: literal-string() != NULL) { On 2014/05/08 12:52:08, rossberg wrote: Would be nicer to have an IsString predicate on Literal, so that you don't have to rely on such internals here. The
[v8-dev] Re: WIP: Parser: delay string internalization. (issue 231073002)
and closing this one; the new CL is https://codereview.chromium.org/314603004 https://codereview.chromium.org/231073002/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from it, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] Re: Add dependency on buildtools repo (issue 308353002)
LGTM https://codereview.chromium.org/308353002/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from it, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] Re: Don't clear exception pending message if we have externally TryCatch and finally on top of stack. (issue 306463002)
On 2014/06/02 15:21:16, Yang wrote: Looking good. I got some comments. And I would like to add mstarzinger@ for a second opinion. https://chromiumcodereview.appspot.com/306463002/diff/60001/src/isolate.cc File src/isolate.cc (right): https://chromiumcodereview.appspot.com/306463002/diff/60001/src/isolate.cc#newcode1154 src/isolate.cc:1154: return false; Can we simply return an expression? Yes, we can. Done. https://chromiumcodereview.appspot.com/306463002/diff/60001/src/isolate.cc#newcode1192 src/isolate.cc:1192: bool canClearMessage = PropagatePendingExceptionToExternalTryCatch(); please name this 'can_clear_message' instead. Done. https://chromiumcodereview.appspot.com/306463002/diff/60001/src/isolate.cc#newcode1220 src/isolate.cc:1220: clear_pending_message(); no line break here. Done. On 2014/06/02 15:31:27, vsevik wrote: https://chromiumcodereview.appspot.com/306463002/diff/60001/src/isolate.cc File src/isolate.cc (left): https://chromiumcodereview.appspot.com/306463002/diff/60001/src/isolate.cc#oldcode1157 src/isolate.cc:1157: if (!is_catchable_by_javascript(pending_exception())) { Looking at this once again I think this check is still needed. Unfortunately you will probably need to extract another function. :( I've reused function is Isolate::is_catchable_by_javascript. Done. https://codereview.chromium.org/306463002/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from it, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] Parser: Delay internalizing strings and values. (issue 314603004)
Reviewers: rossberg, Message: (rossberg, this is for you) (ulan, mstarzinger, dcarney, jochen fyi) Here's a new version of the delay internalizing strings. I'd like another round of comments... cctest, mjsunit, webkit, mozilla and test262 pass. I'm planning to add more tests for function name inferring. Speed-wise it's a bit slower than baseline (5-7% on my non-lazy test data, both on mobile and on desktop). That small regression shouldn't regress any benchmarks. Description: Parser: Delay internalizing strings and values. This is needed so that we can run Parser on a non-main thread (independent of the Isolate and the V8 heap). BUG= Please review this at https://codereview.chromium.org/314603004/ SVN Base: https://v8.googlecode.com/svn/branches/bleeding_edge Affected files (+1426, -621 lines): M include/v8.h M src/ast.h M src/ast.cc A src/ast-string-table.h A src/ast-string-table.cc M src/compiler.h M src/compiler.cc M src/func-name-inferrer.h M src/func-name-inferrer.cc M src/heap.h M src/interface.h M src/interface.cc M src/list.h M src/objects.cc M src/parser.h M src/parser.cc M src/preparser.h M src/prettyprinter.h M src/prettyprinter.cc M src/rewriter.cc M src/scanner.h M src/scanner.cc M src/scopeinfo.cc M src/scopes.h M src/scopes.cc M src/utils.h M src/utils.cc M src/variables.h M src/variables.cc M test/cctest/test-ast.cc M test/cctest/test-parsing.cc M tools/gyp/v8.gyp M tools/parser-shell.cc -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from it, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] Re: Fix leak in debug mirror cache. (issue 296953005)
On 2014/06/03 08:12:46, Yang wrote: https://codereview.chromium.org/296953005/diff/40001/src/debug-debugger.js File src/debug-debugger.js (right): https://codereview.chromium.org/296953005/diff/40001/src/debug-debugger.js#newcode491 src/debug-debugger.js:491: return %DebugGetLoadedScripts(); On 2014/06/03 08:07:21, yurys wrote: This change looks wrong to me. By design of V8 debugging protocol mirror cache should be kept while we are staying on a breakpoint so that the client could access corresponding object by its mirror id. After this change, however, the cache will be cleared on any call to GetLoadedScripts() and the remote id will become invalid. In case of blink the problem is that we create mirror objects not only when we stay on a breakpoint and I'm not sure we clear them properly. Also we don't use id-mirror map in blink and I believe a right way to fix this would be to disable mirror cache in blink completely. WDYT? Not sure I understand. Which code path does blink use to get loaded scripts? Blink adds a bunch of convenience methods into the debug context all of which are defined in DebuggerScript.js [1] and they leverage mirror infrastructure to inspected user objects (one important difference compared to v8 debugger as I said is that we never need to resolve id into morror object so the cache is useless in case of Blink). To collect current scripts PageScriptDebugServer.cpp calls [2] DebuggerScript.getScripts() which in turn calls Debug.scripts() [3] but all of this happens without entering debugger as simple function call using v8 api. As we don't enter debugger in that case ClearMirrorCache may never be called. This might be a bug in this particular case and we should use v8::Debug::Call for invoking DebuggerScript.getScripts() to make sure EnterDebugger is called and mirror cache is cleared after the call. But in general to avoid bugs like this we could skip mirror cache for the mirrors created by Blink. [1] https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit/Source/bindings/v8/DebuggerScript.js [2] https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit/Source/bindings/v8/PageScriptDebugServer.cppq=PageScriptDebugServer.cppsq=package:chromiumtype=csl=132 [3] https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit/Source/bindings/v8/DebuggerScript.jsq=getScriptssq=package:chromiumtype=csl=132 https://codereview.chromium.org/296953005/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from it, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] Rename new_space_dominator to dominator since dominators can also be in old space. (issue 312713002)
Reviewers: Benedikt Meurer, Description: Rename new_space_dominator to dominator since dominators can also be in old space. BUG= Please review this at https://codereview.chromium.org/312713002/ SVN Base: https://v8.googlecode.com/svn/branches/bleeding_edge Affected files (+11, -13 lines): M src/hydrogen-instructions.h Index: src/hydrogen-instructions.h diff --git a/src/hydrogen-instructions.h b/src/hydrogen-instructions.h index a22da110cb55ea19b6794f68c90c7eb0f3e75028..a6e974e49fa9b238a801d23bf5106ffd43cf4dec 100644 --- a/src/hydrogen-instructions.h +++ b/src/hydrogen-instructions.h @@ -6681,7 +6681,7 @@ class HStoreNamedField V8_FINAL : public HTemplateInstruction3 { HValue* dominator) V8_OVERRIDE { ASSERT(side_effect == kNewSpacePromotion); if (!FLAG_use_write_barrier_elimination) return false; -new_space_dominator_ = dominator; +dominator_ = dominator; return false; } virtual void PrintDataTo(StringStream* stream) V8_OVERRIDE; @@ -6691,7 +6691,7 @@ class HStoreNamedField V8_FINAL : public HTemplateInstruction3 { HValue* transition() const { return OperandAt(2); } HObjectAccess access() const { return access_; } - HValue* new_space_dominator() const { return new_space_dominator_; } + HValue* dominator() const { return dominator_; } bool has_transition() const { return has_transition_; } StoreFieldOrKeyedMode store_mode() const { return store_mode_; } @@ -6718,13 +6718,12 @@ class HStoreNamedField V8_FINAL : public HTemplateInstruction3 { if (field_representation().IsInteger32()) return false; if (field_representation().IsExternal()) return false; return StoringValueNeedsWriteBarrier(value()) -ReceiverObjectNeedsWriteBarrier(object(), value(), -new_space_dominator()); +ReceiverObjectNeedsWriteBarrier(object(), value(), dominator()); } bool NeedsWriteBarrierForMap() { return ReceiverObjectNeedsWriteBarrier(object(), transition(), - new_space_dominator()); + dominator()); } SmiCheck SmiCheckForWriteBarrier() const { @@ -6760,7 +6759,7 @@ class HStoreNamedField V8_FINAL : public HTemplateInstruction3 { HValue* val, StoreFieldOrKeyedMode store_mode = INITIALIZING_STORE) : access_(access), -new_space_dominator_(NULL), +dominator_(NULL), has_transition_(false), store_mode_(store_mode) { // Stores to a non existing in-object property are allowed only to the @@ -6774,7 +6773,7 @@ class HStoreNamedField V8_FINAL : public HTemplateInstruction3 { } HObjectAccess access_; - HValue* new_space_dominator_; + HValue* dominator_; bool has_transition_ : 1; StoreFieldOrKeyedMode store_mode_ : 1; }; @@ -6922,19 +6921,18 @@ class HStoreKeyed V8_FINAL virtual bool HandleSideEffectDominator(GVNFlag side_effect, HValue* dominator) V8_OVERRIDE { ASSERT(side_effect == kNewSpacePromotion); -new_space_dominator_ = dominator; +dominator_ = dominator; return false; } - HValue* new_space_dominator() const { return new_space_dominator_; } + HValue* dominator() const { return dominator_; } bool NeedsWriteBarrier() { if (value_is_smi()) { return false; } else { return StoringValueNeedsWriteBarrier(value()) - ReceiverObjectNeedsWriteBarrier(elements(), value(), - new_space_dominator()); + ReceiverObjectNeedsWriteBarrier(elements(), value(), dominator()); } } @@ -6956,7 +6954,7 @@ class HStoreKeyed V8_FINAL is_dehoisted_(false), is_uninitialized_(false), store_mode_(store_mode), - new_space_dominator_(NULL) { + dominator_(NULL) { SetOperandAt(0, obj); SetOperandAt(1, key); SetOperandAt(2, val); @@ -6996,7 +6994,7 @@ class HStoreKeyed V8_FINAL bool is_dehoisted_ : 1; bool is_uninitialized_ : 1; StoreFieldOrKeyedMode store_mode_: 1; - HValue* new_space_dominator_; + HValue* dominator_; }; -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from it, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] Re: Rename new_space_dominator to dominator since dominators can also be in old space. (issue 312713002)
LGTM https://codereview.chromium.org/312713002/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from it, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] [v8] r21628 committed - Fix test...
Revision: 21628 Author: rossb...@chromium.org Date: Tue Jun 3 09:34:29 2014 UTC Log: Fix test R=bmeu...@chromium.org BUG= Review URL: https://codereview.chromium.org/306353002 http://code.google.com/p/v8/source/detail?r=21628 Modified: /branches/bleeding_edge/test/cctest/test-types.cc === --- /branches/bleeding_edge/test/cctest/test-types.cc Tue Jun 3 08:12:43 2014 UTC +++ /branches/bleeding_edge/test/cctest/test-types.cc Tue Jun 3 09:34:29 2014 UTC @@ -1160,13 +1160,13 @@ } } -// T1-Maybe(T2) iff Intersect(T1, T2) inhabited +// T1-Maybe(T2) implies Intersect(T1, T2) inhabited for (TypeIterator it1 = T.types.begin(); it1 != T.types.end(); ++it1) { for (TypeIterator it2 = T.types.begin(); it2 != T.types.end(); ++it2) { TypeHandle type1 = *it1; TypeHandle type2 = *it2; TypeHandle intersect12 = T.Intersect(type1, type2); -CHECK(type1-Maybe(type2) == intersect12-IsInhabited()); +CHECK(!type1-Maybe(type2) || intersect12-IsInhabited()); } } -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from it, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] Re: Fix test (issue 306353002)
Committed patchset #1 manually as r21628 (tree was closed). https://codereview.chromium.org/306353002/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from it, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] Re: Fix leak in debug mirror cache. (issue 296953005)
So, could we simply skip the cache if the debugger is not entered? I would then set a flag on the global object of the debug context after entering the debugger, which guards caching. Leaving the debugger would clear the cache and clear the flag. Since blink never enters the debugger, the mirror cache is not active. I would then remove the ClearMirrorCache call when getting scripts. WDYT? Yang On Tue, Jun 3, 2014 at 11:29 AM, yu...@chromium.org wrote: On 2014/06/03 08:12:46, Yang wrote: https://codereview.chromium.org/296953005/diff/40001/src/ debug-debugger.js File src/debug-debugger.js (right): https://codereview.chromium.org/296953005/diff/40001/src/ debug-debugger.js#newcode491 src/debug-debugger.js:491: return %DebugGetLoadedScripts(); On 2014/06/03 08:07:21, yurys wrote: This change looks wrong to me. By design of V8 debugging protocol mirror cache should be kept while we are staying on a breakpoint so that the client could access corresponding object by its mirror id. After this change, however, the cache will be cleared on any call to GetLoadedScripts() and the remote id will become invalid. In case of blink the problem is that we create mirror objects not only when we stay on a breakpoint and I'm not sure we clear them properly. Also we don't use id-mirror map in blink and I believe a right way to fix this would be to disable mirror cache in blink completely. WDYT? Not sure I understand. Which code path does blink use to get loaded scripts? Blink adds a bunch of convenience methods into the debug context all of which are defined in DebuggerScript.js [1] and they leverage mirror infrastructure to inspected user objects (one important difference compared to v8 debugger as I said is that we never need to resolve id into morror object so the cache is useless in case of Blink). To collect current scripts PageScriptDebugServer.cpp calls [2] DebuggerScript.getScripts() which in turn calls Debug.scripts() [3] but all of this happens without entering debugger as simple function call using v8 api. As we don't enter debugger in that case ClearMirrorCache may never be called. This might be a bug in this particular case and we should use v8::Debug::Call for invoking DebuggerScript.getScripts() to make sure EnterDebugger is called and mirror cache is cleared after the call. But in general to avoid bugs like this we could skip mirror cache for the mirrors created by Blink. [1] https://code.google.com/p/chromium/codesearch#chromium/ src/third_party/WebKit/Source/bindings/v8/DebuggerScript.js [2] https://code.google.com/p/chromium/codesearch#chromium/ src/third_party/WebKit/Source/bindings/v8/PageScriptDebugServer.cppq= PageScriptDebugServer.cppsq=package:chromiumtype=csl=132 [3] https://code.google.com/p/chromium/codesearch#chromium/ src/third_party/WebKit/Source/bindings/v8/DebuggerScript.js q=getScriptssq=package:chromiumtype=csl=132 https://codereview.chromium.org/296953005/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from it, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] Remove non-incremental idle notification handler. (issue 315553002)
Reviewers: Hannes Payer, Message: PTAL. This code is probably never used in practice, since --incremental-marking is on by default. Description: Remove non-incremental idle notification handler. Please review this at https://codereview.chromium.org/315553002/ SVN Base: https://v8.googlecode.com/svn/branches/bleeding_edge Affected files (+1, -71 lines): M src/heap.h M src/heap.cc Index: src/heap.cc diff --git a/src/heap.cc b/src/heap.cc index ec058828e664cec507581ffe40db26b9edcbe2a7..dd00f9e8ab0be9561379371bde27120f2a138588 100644 --- a/src/heap.cc +++ b/src/heap.cc @@ -114,9 +114,6 @@ Heap::Heap() store_buffer_(this), marking_(this), incremental_marking_(this), - number_idle_notifications_(0), - last_idle_notification_gc_count_(0), - last_idle_notification_gc_count_init_(false), mark_sweeps_since_idle_round_started_(0), gc_count_at_last_idle_gc_(0), scavenges_since_last_idle_round_(kIdleScavengeThreshold), @@ -4337,7 +4334,7 @@ bool Heap::IdleNotification(int hint) { } if (!FLAG_incremental_marking || isolate_-serializer_enabled()) { -return IdleGlobalGC(); +return true; } // By doing small chunks of GC work in each IdleNotification, @@ -4394,66 +4391,6 @@ bool Heap::IdleNotification(int hint) { } -bool Heap::IdleGlobalGC() { - static const int kIdlesBeforeScavenge = 4; - static const int kIdlesBeforeMarkSweep = 7; - static const int kIdlesBeforeMarkCompact = 8; - static const int kMaxIdleCount = kIdlesBeforeMarkCompact + 1; - static const unsigned int kGCsBetweenCleanup = 4; - - if (!last_idle_notification_gc_count_init_) { -last_idle_notification_gc_count_ = gc_count_; -last_idle_notification_gc_count_init_ = true; - } - - bool uncommit = true; - bool finished = false; - - // Reset the number of idle notifications received when a number of - // GCs have taken place. This allows another round of cleanup based - // on idle notifications if enough work has been carried out to - // provoke a number of garbage collections. - if (gc_count_ - last_idle_notification_gc_count_ kGCsBetweenCleanup) { -number_idle_notifications_ = -Min(number_idle_notifications_ + 1, kMaxIdleCount); - } else { -number_idle_notifications_ = 0; -last_idle_notification_gc_count_ = gc_count_; - } - - if (number_idle_notifications_ == kIdlesBeforeScavenge) { -CollectGarbage(NEW_SPACE, idle notification); -new_space_.Shrink(); -last_idle_notification_gc_count_ = gc_count_; - } else if (number_idle_notifications_ == kIdlesBeforeMarkSweep) { -// Before doing the mark-sweep collections we clear the -// compilation cache to avoid hanging on to source code and -// generated code for cached functions. -isolate_-compilation_cache()-Clear(); - -CollectAllGarbage(kReduceMemoryFootprintMask, idle notification); -new_space_.Shrink(); -last_idle_notification_gc_count_ = gc_count_; - - } else if (number_idle_notifications_ == kIdlesBeforeMarkCompact) { -CollectAllGarbage(kReduceMemoryFootprintMask, idle notification); -new_space_.Shrink(); -last_idle_notification_gc_count_ = gc_count_; -number_idle_notifications_ = 0; -finished = true; - } else if (number_idle_notifications_ kIdlesBeforeMarkCompact) { -// If we have received more than kIdlesBeforeMarkCompact idle -// notifications we do not perform any cleanup because we don't -// expect to gain much by doing so. -finished = true; - } - - if (uncommit) UncommitFromSpace(); - - return finished; -} - - #ifdef DEBUG void Heap::Print() { Index: src/heap.h diff --git a/src/heap.h b/src/heap.h index b465ca47c485bb7223cd3f6713dc230258b000fe..33d7bf4fecc66694c342ea48e65a5005dc9e3331 100644 --- a/src/heap.h +++ b/src/heap.h @@ -2062,9 +2062,6 @@ class Heap { return heap_size_mb / kMbPerMs; } - // Returns true if no more GC work is left. - bool IdleGlobalGC(); - void AdvanceIdleIncrementalMarking(intptr_t step_size); void ClearObjectStats(bool clear_last_time_stats = false); @@ -2119,10 +2116,6 @@ class Heap { IncrementalMarking incremental_marking_; - int number_idle_notifications_; - unsigned int last_idle_notification_gc_count_; - bool last_idle_notification_gc_count_init_; - int mark_sweeps_since_idle_round_started_; unsigned int gc_count_at_last_idle_gc_; int scavenges_since_last_idle_round_; -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from it, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] Field layout in class Arguments is incompatible w\ 64-bit archs. (issue 308003018)
lgtm https://codereview.chromium.org/308003018/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from it, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] Re: Improve write barriers in optimized code. (issue 297763006)
lgtm https://codereview.chromium.org/297763006/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from it, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] Re: Rename new_space_dominator to dominator since dominators can also be in old space. (issue 312713002)
Committed patchset #2 manually as r21629 (tree was closed). https://codereview.chromium.org/312713002/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from it, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] [v8] r21629 committed - Rename new_space_dominator to dominator since dominators can also be i...
Revision: 21629 Author: hpa...@chromium.org Date: Tue Jun 3 10:40:36 2014 UTC Log: Rename new_space_dominator to dominator since dominators can also be in old space. BUG= R=bmeu...@chromium.org Review URL: https://codereview.chromium.org/312713002 http://code.google.com/p/v8/source/detail?r=21629 Modified: /branches/bleeding_edge/src/hydrogen-instructions.h === --- /branches/bleeding_edge/src/hydrogen-instructions.h Tue Jun 3 08:12:43 2014 UTC +++ /branches/bleeding_edge/src/hydrogen-instructions.h Tue Jun 3 10:40:36 2014 UTC @@ -6681,7 +6681,7 @@ HValue* dominator) V8_OVERRIDE { ASSERT(side_effect == kNewSpacePromotion); if (!FLAG_use_write_barrier_elimination) return false; -new_space_dominator_ = dominator; +dominator_ = dominator; return false; } virtual void PrintDataTo(StringStream* stream) V8_OVERRIDE; @@ -6691,7 +6691,7 @@ HValue* transition() const { return OperandAt(2); } HObjectAccess access() const { return access_; } - HValue* new_space_dominator() const { return new_space_dominator_; } + HValue* dominator() const { return dominator_; } bool has_transition() const { return has_transition_; } StoreFieldOrKeyedMode store_mode() const { return store_mode_; } @@ -6718,13 +6718,12 @@ if (field_representation().IsInteger32()) return false; if (field_representation().IsExternal()) return false; return StoringValueNeedsWriteBarrier(value()) -ReceiverObjectNeedsWriteBarrier(object(), value(), -new_space_dominator()); +ReceiverObjectNeedsWriteBarrier(object(), value(), dominator()); } bool NeedsWriteBarrierForMap() { return ReceiverObjectNeedsWriteBarrier(object(), transition(), - new_space_dominator()); + dominator()); } SmiCheck SmiCheckForWriteBarrier() const { @@ -6760,7 +6759,7 @@ HValue* val, StoreFieldOrKeyedMode store_mode = INITIALIZING_STORE) : access_(access), -new_space_dominator_(NULL), +dominator_(NULL), has_transition_(false), store_mode_(store_mode) { // Stores to a non existing in-object property are allowed only to the @@ -6774,7 +6773,7 @@ } HObjectAccess access_; - HValue* new_space_dominator_; + HValue* dominator_; bool has_transition_ : 1; StoreFieldOrKeyedMode store_mode_ : 1; }; @@ -6922,19 +6921,18 @@ virtual bool HandleSideEffectDominator(GVNFlag side_effect, HValue* dominator) V8_OVERRIDE { ASSERT(side_effect == kNewSpacePromotion); -new_space_dominator_ = dominator; +dominator_ = dominator; return false; } - HValue* new_space_dominator() const { return new_space_dominator_; } + HValue* dominator() const { return dominator_; } bool NeedsWriteBarrier() { if (value_is_smi()) { return false; } else { return StoringValueNeedsWriteBarrier(value()) - ReceiverObjectNeedsWriteBarrier(elements(), value(), - new_space_dominator()); + ReceiverObjectNeedsWriteBarrier(elements(), value(), dominator()); } } @@ -6956,7 +6954,7 @@ is_dehoisted_(false), is_uninitialized_(false), store_mode_(store_mode), - new_space_dominator_(NULL) { + dominator_(NULL) { SetOperandAt(0, obj); SetOperandAt(1, key); SetOperandAt(2, val); @@ -6996,7 +6994,7 @@ bool is_dehoisted_ : 1; bool is_uninitialized_ : 1; StoreFieldOrKeyedMode store_mode_: 1; - HValue* new_space_dominator_; + HValue* dominator_; }; -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from it, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] Re: Add test that inherited properties are preserved for synthetic change records (issue 309573002)
lgtm https://codereview.chromium.org/309573002/diff/1/test/mjsunit/es7/object-observe.js File test/mjsunit/es7/object-observe.js (right): https://codereview.chromium.org/309573002/diff/1/test/mjsunit/es7/object-observe.js#newcode225 test/mjsunit/es7/object-observe.js:225: var rec2 = Object.create(proto); On 2014/06/02 18:32:20, rafaelw wrote: removed. On 2014/06/02 09:14:15, rossberg wrote: I don't understand, this is never used. Ah, OK. I wasn't sure if the intention was to use it in place of rec in the lines that followed. https://codereview.chromium.org/309573002/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from it, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] Re: Add v8::Promise::Then. (issue 314553002)
LGTM, although I don't buy the premise that this is what Blink should use. https://codereview.chromium.org/314553002/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from it, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] Re: Improve write barriers in optimized code. (issue 297763006)
Committed patchset #5 manually as r21630 (tree was closed). https://codereview.chromium.org/297763006/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from it, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] [v8] r21630 committed - Improve write barriers in optimized code....
Revision: 21630 Author: bmeu...@chromium.org Date: Tue Jun 3 10:59:11 2014 UTC Log: Improve write barriers in optimized code. Use a cheaper RecordWriteForMap() to update the write barrier for maps. And skip the value check in RecordWriteField() when we statically know that the value is in new space (and therefore has pointers to here are interesting flag set). R=hpa...@chromium.org Review URL: https://codereview.chromium.org/297763006 http://code.google.com/p/v8/source/detail?r=21630 Modified: /branches/bleeding_edge/src/arm/lithium-codegen-arm.cc /branches/bleeding_edge/src/arm/macro-assembler-arm.cc /branches/bleeding_edge/src/arm/macro-assembler-arm.h /branches/bleeding_edge/src/arm64/lithium-codegen-arm64.cc /branches/bleeding_edge/src/arm64/macro-assembler-arm64.cc /branches/bleeding_edge/src/arm64/macro-assembler-arm64.h /branches/bleeding_edge/src/hydrogen-instructions.h /branches/bleeding_edge/src/ia32/lithium-codegen-ia32.cc /branches/bleeding_edge/src/ia32/macro-assembler-ia32.cc /branches/bleeding_edge/src/ia32/macro-assembler-ia32.h /branches/bleeding_edge/src/x64/lithium-codegen-x64.cc /branches/bleeding_edge/src/x64/macro-assembler-x64.cc /branches/bleeding_edge/src/x64/macro-assembler-x64.h === --- /branches/bleeding_edge/src/arm/lithium-codegen-arm.cc Tue Jun 3 08:12:43 2014 UTC +++ /branches/bleeding_edge/src/arm/lithium-codegen-arm.cc Tue Jun 3 10:59:11 2014 UTC @@ -4084,14 +4084,11 @@ if (instr-hydrogen()-NeedsWriteBarrierForMap()) { Register temp = ToRegister(instr-temp()); // Update the write barrier for the map field. - __ RecordWriteField(object, - HeapObject::kMapOffset, - scratch, - temp, - GetLinkRegisterState(), - kSaveFPRegs, - OMIT_REMEMBERED_SET, - OMIT_SMI_CHECK); + __ RecordWriteForMap(object, + scratch, + temp, + GetLinkRegisterState(), + kSaveFPRegs); } } @@ -4109,7 +4106,8 @@ GetLinkRegisterState(), kSaveFPRegs, EMIT_REMEMBERED_SET, - instr-hydrogen()-SmiCheckForWriteBarrier()); + instr-hydrogen()-SmiCheckForWriteBarrier(), + instr-hydrogen()-PointersToHereCheckForValue()); } } else { __ ldr(scratch, FieldMemOperand(object, JSObject::kPropertiesOffset)); @@ -4125,7 +4123,8 @@ GetLinkRegisterState(), kSaveFPRegs, EMIT_REMEMBERED_SET, - instr-hydrogen()-SmiCheckForWriteBarrier()); + instr-hydrogen()-SmiCheckForWriteBarrier(), + instr-hydrogen()-PointersToHereCheckForValue()); } } } @@ -4335,7 +4334,8 @@ GetLinkRegisterState(), kSaveFPRegs, EMIT_REMEMBERED_SET, - check_needed); + check_needed, + instr-hydrogen()-PointersToHereCheckForValue()); } } @@ -4384,8 +4384,11 @@ __ mov(new_map_reg, Operand(to_map)); __ str(new_map_reg, FieldMemOperand(object_reg, HeapObject::kMapOffset)); // Write barrier. -__ RecordWriteField(object_reg, HeapObject::kMapOffset, new_map_reg, -scratch, GetLinkRegisterState(), kDontSaveFPRegs); +__ RecordWriteForMap(object_reg, + new_map_reg, + scratch, + GetLinkRegisterState(), + kDontSaveFPRegs); } else { ASSERT(ToRegister(instr-context()).is(cp)); ASSERT(object_reg.is(r0)); === --- /branches/bleeding_edge/src/arm/macro-assembler-arm.cc Tue Jun 3 08:12:43 2014 UTC +++ /branches/bleeding_edge/src/arm/macro-assembler-arm.cc Tue Jun 3 10:59:11 2014 UTC @@ -453,7 +453,8 @@ LinkRegisterStatus lr_status, SaveFPRegsMode save_fp, RememberedSetAction remembered_set_action, -SmiCheck smi_check) { +SmiCheck smi_check, +PointersToHereCheck pointers_to_here_check_for_value) { // First, check if a write barrier is even needed. The tests below // catch stores of Smis. Label done; @@ -482,7 +483,8 @@ lr_status, save_fp, remembered_set_action, - OMIT_SMI_CHECK); + OMIT_SMI_CHECK, + pointers_to_here_check_for_value); bind(done); @@ -493,24 +495,101 @@ mov(dst, Operand(BitCastint32_t(kZapValue + 8))); } } + + +// Will clobber 4 registers: object, map, dst, ip. The +// register 'object' contains a heap
[v8-dev] Update include paths for gcmole (issue 310733005)
Reviewers: mvstanton, Description: Update include paths for gcmole TBR=mvstan...@chromium.org LOG=n BUG=none Please review this at https://codereview.chromium.org/310733005/ SVN Base: https://v8.googlecode.com/svn/branches/bleeding_edge Affected files (+1, -1 lines): M tools/gcmole/gcmole.lua Index: tools/gcmole/gcmole.lua diff --git a/tools/gcmole/gcmole.lua b/tools/gcmole/gcmole.lua index f1980a459611717744061c6d94b4f1592c4a7306..706f4de86fbc30a42c3dce8e6185cd022c3c68d8 100644 --- a/tools/gcmole/gcmole.lua +++ b/tools/gcmole/gcmole.lua @@ -104,7 +104,7 @@ local function MakeClangCommandLine(plugin, plugin_args, triple, arch_define) .. -D .. arch_define .. -DENABLE_DEBUGGER_SUPPORT .. -DV8_I18N_SUPPORT - .. -Isrc + .. -I./ .. -Ithird_party/icu/source/common .. -Ithird_party/icu/source/i18n end -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from it, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] Re: Update include paths for gcmole (issue 310733005)
Committed patchset #1 manually as r21631 (tree was closed). https://codereview.chromium.org/310733005/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from it, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] [v8] r21631 committed - Update include paths for gcmole...
Revision: 21631 Author: joc...@chromium.org Date: Tue Jun 3 11:01:35 2014 UTC Log: Update include paths for gcmole TBR=mvstan...@chromium.org LOG=n BUG=none Review URL: https://codereview.chromium.org/310733005 http://code.google.com/p/v8/source/detail?r=21631 Modified: /branches/bleeding_edge/tools/gcmole/gcmole.lua === --- /branches/bleeding_edge/tools/gcmole/gcmole.lua Thu Apr 17 11:57:08 2014 UTC +++ /branches/bleeding_edge/tools/gcmole/gcmole.lua Tue Jun 3 11:01:35 2014 UTC @@ -104,7 +104,7 @@ .. -D .. arch_define .. -DENABLE_DEBUGGER_SUPPORT .. -DV8_I18N_SUPPORT - .. -Isrc + .. -I./ .. -Ithird_party/icu/source/common .. -Ithird_party/icu/source/i18n end -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from it, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] Re: ARM64: Avoid regeneration of constants. (issue 308313002)
On 2014/06/03 08:27:25, Alexandre Rames wrote: On 2014/06/03 08:01:16, danno wrote: Actually, the right thing to do is to treat constants as normal values and teach the register allocator how to split constant ranges and rematerialize on demand without spilling. This is something that Jaro and Benedikt have thought about recently, and I think this is generally a cleaner way to approach the problem than the ad-hoc EmitAsUses approach which is just a brittle heuristic. I did a bit of work on this as a follow-up for this patch: I augmented lithium instructions with a method indicating if it was better to regenerate the value (re-visit the instruction) instead of spilling and restoring it. LiveRanges and LOperands kept track of which lithium instruction output they referred to. The gap resolver skips spilling moves when appropriate, and instead of moving from the stack re-visits the lithium instruction. I think there is some more work to do around that, but if you are interested I could upload this so you can have a look. Yes, it would be great if you could share your work. (I am currently working on a different optimisation in the register allocator, but I was thinking about doing the rematerialization, too.) https://codereview.chromium.org/308313002/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from it, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] Re: Remove non-incremental idle notification handler. (issue 315553002)
lgtm https://codereview.chromium.org/315553002/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from it, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] Re: Fix leak in debug mirror cache. (issue 296953005)
On Tue, Jun 3, 2014 at 1:37 PM, Yang Guo yang...@chromium.org wrote: So, could we simply skip the cache if the debugger is not entered? I would then set a flag on the global object of the debug context after entering the debugger, which guards caching. Leaving the debugger would clear the cache and clear the flag. We don't need to tie this to the debugger entrance, we just need a global flag for disabling mirror cache. We would set that flag in DebuggerScript.js initialization code. Since blink never enters the debugger, the mirror cache is not active. I would then remove the ClearMirrorCache call when getting scripts. Blink does enter the debugger in some cases, e.g. when doing evals on pause but we still don't need the cache in those cases. WDYT? Yang On Tue, Jun 3, 2014 at 11:29 AM, yu...@chromium.org wrote: On 2014/06/03 08:12:46, Yang wrote: https://codereview.chromium.org/296953005/diff/40001/src/ debug-debugger.js File src/debug-debugger.js (right): https://codereview.chromium.org/296953005/diff/40001/src/ debug-debugger.js#newcode491 src/debug-debugger.js:491: return %DebugGetLoadedScripts(); On 2014/06/03 08:07:21, yurys wrote: This change looks wrong to me. By design of V8 debugging protocol mirror cache should be kept while we are staying on a breakpoint so that the client could access corresponding object by its mirror id. After this change, however, the cache will be cleared on any call to GetLoadedScripts() and the remote id will become invalid. In case of blink the problem is that we create mirror objects not only when we stay on a breakpoint and I'm not sure we clear them properly. Also we don't use id-mirror map in blink and I believe a right way to fix this would be to disable mirror cache in blink completely. WDYT? Not sure I understand. Which code path does blink use to get loaded scripts? Blink adds a bunch of convenience methods into the debug context all of which are defined in DebuggerScript.js [1] and they leverage mirror infrastructure to inspected user objects (one important difference compared to v8 debugger as I said is that we never need to resolve id into morror object so the cache is useless in case of Blink). To collect current scripts PageScriptDebugServer.cpp calls [2] DebuggerScript.getScripts() which in turn calls Debug.scripts() [3] but all of this happens without entering debugger as simple function call using v8 api. As we don't enter debugger in that case ClearMirrorCache may never be called. This might be a bug in this particular case and we should use v8::Debug::Call for invoking DebuggerScript.getScripts() to make sure EnterDebugger is called and mirror cache is cleared after the call. But in general to avoid bugs like this we could skip mirror cache for the mirrors created by Blink. [1] https://code.google.com/p/chromium/codesearch#chromium/ src/third_party/WebKit/Source/bindings/v8/DebuggerScript.js [2] https://code.google.com/p/chromium/codesearch#chromium/ src/third_party/WebKit/Source/bindings/v8/PageScriptDebugServer.cppq= PageScriptDebugServer.cppsq=package:chromiumtype=csl=132 [3] https://code.google.com/p/chromium/codesearch#chromium/ src/third_party/WebKit/Source/bindings/v8/DebuggerScript.js q=getScriptssq=package:chromiumtype=csl=132 https://codereview.chromium.org/296953005/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from it, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] Visit encountered JSWeakCollection list during scavenging. (issue 310783003)
Reviewers: Hannes Payer, Igor Sheludko, Message: Igor: Could you please check against your repro? Hannes: PTAL. Description: Visit encountered JSWeakCollection list during scavenging. R=hpa...@chromium.org BUG=chromium:380068 LOG=N Please review this at https://codereview.chromium.org/310783003/ SVN Base: https://v8.googlecode.com/svn/branches/bleeding_edge Affected files (+21, -15 lines): M src/heap.h M src/heap.cc M src/mark-compact.h M src/mark-compact.cc M src/objects-visiting-inl.h Index: src/heap.cc diff --git a/src/heap.cc b/src/heap.cc index ff7a9ff12f20d6f7e37751f7d2bd91fb26939684..a530c396bedda23beec01b349f57c62f3760c944 100644 --- a/src/heap.cc +++ b/src/heap.cc @@ -145,6 +145,7 @@ Heap::Heap() set_native_contexts_list(NULL); set_array_buffers_list(Smi::FromInt(0)); set_allocation_sites_list(Smi::FromInt(0)); + set_encountered_weak_collections(Smi::FromInt(0)); // Put a dummy entry in the remembered pages so we can find the list the // minidump even if there are no real unmapped pages. RememberUnmappedPage(NULL, false); @@ -1508,6 +1509,9 @@ void Heap::Scavenge() { } } + // Copy objects reachable from the encountered weak collections list. + scavenge_visitor.VisitPointer(encountered_weak_collections_); + // Copy objects reachable from the code flushing candidates list. MarkCompactCollector* collector = mark_compact_collector(); if (collector-is_code_flushing_enabled()) { Index: src/heap.h diff --git a/src/heap.h b/src/heap.h index 8956e3c56b50f31af04c57a3caf9669d4c58f3f2..9e5b6a55253552b979e2315a365189034615b02e 100644 --- a/src/heap.h +++ b/src/heap.h @@ -849,6 +849,13 @@ class Heap { Object* weak_object_to_code_table() { return weak_object_to_code_table_; } + void set_encountered_weak_collections(Object* weak_collection) { +encountered_weak_collections_ = weak_collection; + } + Object* encountered_weak_collections() const { +return encountered_weak_collections_; + } + // Number of mark-sweeps. unsigned int ms_count() { return ms_count_; } @@ -1605,6 +1612,11 @@ class Heap { // start. Object* weak_object_to_code_table_; + // List of encountered weak collections (JSWeakMap and JSWeakSet) during + // marking. It is initialized during marking, destroyed after marking and + // contains Smi(0) while marking is not active. + Object* encountered_weak_collections_; + StoreBufferRebuilder store_buffer_rebuilder_; struct StringTypeTable { Index: src/mark-compact.cc diff --git a/src/mark-compact.cc b/src/mark-compact.cc index fcaad0def2dee36ce5e03c3994e77137ed7a053d..53f884c6af5a86d8de9d643bd02a1e2e067c21bb 100644 --- a/src/mark-compact.cc +++ b/src/mark-compact.cc @@ -51,7 +51,6 @@ MarkCompactCollector::MarkCompactCollector(Heap* heap) : // NOLINT migration_slots_buffer_(NULL), heap_(heap), code_flusher_(NULL), - encountered_weak_collections_(NULL), have_code_to_deoptimize_(false) { } #ifdef VERIFY_HEAP @@ -2738,7 +2737,7 @@ void MarkCompactCollector::ClearNonLiveDependentCode(DependentCode* entries) { void MarkCompactCollector::ProcessWeakCollections() { GCTracer::Scope gc_scope(tracer_, GCTracer::Scope::MC_WEAKCOLLECTION_PROCESS); - Object* weak_collection_obj = encountered_weak_collections(); + Object* weak_collection_obj = heap()-encountered_weak_collections(); while (weak_collection_obj != Smi::FromInt(0)) { JSWeakCollection* weak_collection = reinterpret_castJSWeakCollection*(weak_collection_obj); @@ -2765,7 +2764,7 @@ void MarkCompactCollector::ProcessWeakCollections() { void MarkCompactCollector::ClearWeakCollections() { GCTracer::Scope gc_scope(tracer_, GCTracer::Scope::MC_WEAKCOLLECTION_CLEAR); - Object* weak_collection_obj = encountered_weak_collections(); + Object* weak_collection_obj = heap()-encountered_weak_collections(); while (weak_collection_obj != Smi::FromInt(0)) { JSWeakCollection* weak_collection = reinterpret_castJSWeakCollection*(weak_collection_obj); @@ -2782,7 +2781,7 @@ void MarkCompactCollector::ClearWeakCollections() { weak_collection_obj = weak_collection-next(); weak_collection-set_next(heap()-undefined_value()); } - set_encountered_weak_collections(Smi::FromInt(0)); + heap()-set_encountered_weak_collections(Smi::FromInt(0)); } Index: src/mark-compact.h diff --git a/src/mark-compact.h b/src/mark-compact.h index bd34d56c5dd9fbcc153b4e7c72e11069abab58ec..18c8fa1ddf6ba8f1dbcfed63a55049ff1b7fa8b0 100644 --- a/src/mark-compact.h +++ b/src/mark-compact.h @@ -649,13 +649,6 @@ class MarkCompactCollector { bool TryPromoteObject(HeapObject* object, int object_size); - inline Object* encountered_weak_collections() { -return encountered_weak_collections_; - } - inline void set_encountered_weak_collections(Object* weak_collection) { -encountered_weak_collections_ = weak_collection; - } - void InvalidateCode(Code* code); void
[v8-dev] Re: Fix leak in debug mirror cache. (issue 296953005)
The thing is, if the debugger is not active, entering the debugger loads a new debug context every time. You would have to re-disable the mirror cache every time. Is that what you want? Yang On Tue, Jun 3, 2014 at 1:22 PM, Yury Semikhatsky yu...@chromium.org wrote: On Tue, Jun 3, 2014 at 1:37 PM, Yang Guo yang...@chromium.org wrote: So, could we simply skip the cache if the debugger is not entered? I would then set a flag on the global object of the debug context after entering the debugger, which guards caching. Leaving the debugger would clear the cache and clear the flag. We don't need to tie this to the debugger entrance, we just need a global flag for disabling mirror cache. We would set that flag in DebuggerScript.js initialization code. Since blink never enters the debugger, the mirror cache is not active. I would then remove the ClearMirrorCache call when getting scripts. Blink does enter the debugger in some cases, e.g. when doing evals on pause but we still don't need the cache in those cases. WDYT? Yang On Tue, Jun 3, 2014 at 11:29 AM, yu...@chromium.org wrote: On 2014/06/03 08:12:46, Yang wrote: https://codereview.chromium.org/296953005/diff/40001/src/ debug-debugger.js File src/debug-debugger.js (right): https://codereview.chromium.org/296953005/diff/40001/src/ debug-debugger.js#newcode491 src/debug-debugger.js:491: return %DebugGetLoadedScripts(); On 2014/06/03 08:07:21, yurys wrote: This change looks wrong to me. By design of V8 debugging protocol mirror cache should be kept while we are staying on a breakpoint so that the client could access corresponding object by its mirror id. After this change, however, the cache will be cleared on any call to GetLoadedScripts() and the remote id will become invalid. In case of blink the problem is that we create mirror objects not only when we stay on a breakpoint and I'm not sure we clear them properly. Also we don't use id-mirror map in blink and I believe a right way to fix this would be to disable mirror cache in blink completely. WDYT? Not sure I understand. Which code path does blink use to get loaded scripts? Blink adds a bunch of convenience methods into the debug context all of which are defined in DebuggerScript.js [1] and they leverage mirror infrastructure to inspected user objects (one important difference compared to v8 debugger as I said is that we never need to resolve id into morror object so the cache is useless in case of Blink). To collect current scripts PageScriptDebugServer.cpp calls [2] DebuggerScript.getScripts() which in turn calls Debug.scripts() [3] but all of this happens without entering debugger as simple function call using v8 api. As we don't enter debugger in that case ClearMirrorCache may never be called. This might be a bug in this particular case and we should use v8::Debug::Call for invoking DebuggerScript.getScripts() to make sure EnterDebugger is called and mirror cache is cleared after the call. But in general to avoid bugs like this we could skip mirror cache for the mirrors created by Blink. [1] https://code.google.com/p/chromium/codesearch#chromium/ src/third_party/WebKit/Source/bindings/v8/DebuggerScript.js [2] https://code.google.com/p/chromium/codesearch#chromium/ src/third_party/WebKit/Source/bindings/v8/PageScriptDebugServer.cppq= PageScriptDebugServer.cppsq=package:chromiumtype=csl=132 [3] https://code.google.com/p/chromium/codesearch#chromium/ src/third_party/WebKit/Source/bindings/v8/DebuggerScript.js q=getScriptssq=package:chromiumtype=csl=132 https://codereview.chromium.org/296953005/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from it, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] X87: Temporarily skip cctest/test-serialize tests in debug mode (issue 307373002)
Reviewers: danno, Message: Hi Danno, I want to temporarily skip test-serialize tests for x87 port in debug mode. The root cause is the same as the snapshot issue of x87 port. We will enable it after fixing this issue. Thanks -Weiliang Description: X87: Temporarily skip cctest/test-serialize tests in debug mode BUG= Please review this at https://codereview.chromium.org/307373002/ SVN Base: https://v8.googlecode.com/svn/branches/bleeding_edge Affected files (+17, -0 lines): M test/cctest/cctest.status Index: test/cctest/cctest.status diff --git a/test/cctest/cctest.status b/test/cctest/cctest.status index 3179364e3c2129260d8d714cb9c2dbe2946cf330..19771ddf32a2830d696ef89463bd2ce6fb90 100644 --- a/test/cctest/cctest.status +++ b/test/cctest/cctest.status @@ -206,6 +206,23 @@ }], # 'arch == mipsel or arch == mips' ## +['arch == x87', { + + # TODO (weiliang): Enable below tests after fixing the double register + # allocation limit in X87 port. + 'test-serialize/Serialize': [PASS, ['mode == debug', SKIP]], + 'test-serialize/Deserialize': [PASS, ['mode == debug', SKIP]], + 'test-serialize/SerializeTwice': [PASS, ['mode == debug', SKIP]], + 'test-serialize/ContextSerialization': [PASS, ['mode == debug', SKIP]], + 'test-serialize/ContextDeserialization': [PASS, ['mode == debug', SKIP]], + 'test-serialize/PartialDeserialization': [PASS, ['mode == debug', SKIP]], + 'test-serialize/PartialSerialization': [PASS, ['mode == debug', SKIP]], + 'test-serialize/DeserializeAndRunScript2': [PASS, ['mode == debug', SKIP]], + 'test-serialize/DeserializeFromSecondSerializationAndRunScript2': [PASS, ['mode == debug', SKIP]], + 'test-serialize/DeserializeFromSecondSerialization': [PASS, ['mode == debug', SKIP]], +}], # 'arch == x87' + +## ['arch == android_arm or arch == android_ia32', { # Tests crash as there is no /tmp directory in Android. -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from it, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] Revert Reland Make 'name' property on functions configurable. (issue 313583002)
Reviewers: Michael Starzinger, Message: Committed patchset #1 manually as r21632 (tree was closed). Description: Revert Reland Make 'name' property on functions configurable. This reverts commit r21609 due to browser test failures. TBR=mstarzin...@chromium.org Committed: https://code.google.com/p/v8/source/detail?r=21632 Please review this at https://codereview.chromium.org/313583002/ SVN Base: https://v8.googlecode.com/svn/branches/bleeding_edge Affected files (+19, -25 lines): M src/bootstrapper.cc M test/mjsunit/es7/object-observe.js M test/mjsunit/regress/regress-1530.js M test/mjsunit/regress/regress-270142.js Index: src/bootstrapper.cc diff --git a/src/bootstrapper.cc b/src/bootstrapper.cc index d562c65c37c5db3f5a1544722c6d0775d4c88340..9f59bcd74ad030455529be4e6b842cfdd8ab253e 100644 --- a/src/bootstrapper.cc +++ b/src/bootstrapper.cc @@ -386,47 +386,45 @@ void Genesis::SetFunctionInstanceDescriptor( int size = (prototypeMode == DONT_ADD_PROTOTYPE) ? 4 : 5; Map::EnsureDescriptorSlack(map, size); - PropertyAttributes ro_attribs = - static_castPropertyAttributes(DONT_ENUM | DONT_DELETE | READ_ONLY); - PropertyAttributes roc_attribs = - static_castPropertyAttributes(DONT_ENUM | READ_ONLY); + PropertyAttributes attribs = static_castPropertyAttributes( + DONT_ENUM | DONT_DELETE | READ_ONLY); HandleAccessorInfo length = - Accessors::FunctionLengthInfo(isolate(), ro_attribs); + Accessors::FunctionLengthInfo(isolate(), attribs); { // Add length. CallbacksDescriptor d(HandleName(Name::cast(length-name())), - length, ro_attribs); + length, attribs); map-AppendDescriptor(d); } HandleAccessorInfo name = - Accessors::FunctionNameInfo(isolate(), roc_attribs); + Accessors::FunctionNameInfo(isolate(), attribs); { // Add name. CallbacksDescriptor d(HandleName(Name::cast(name-name())), - name, roc_attribs); + name, attribs); map-AppendDescriptor(d); } HandleAccessorInfo args = - Accessors::FunctionArgumentsInfo(isolate(), ro_attribs); + Accessors::FunctionArgumentsInfo(isolate(), attribs); { // Add arguments. CallbacksDescriptor d(HandleName(Name::cast(args-name())), - args, ro_attribs); + args, attribs); map-AppendDescriptor(d); } HandleAccessorInfo caller = - Accessors::FunctionCallerInfo(isolate(), ro_attribs); + Accessors::FunctionCallerInfo(isolate(), attribs); { // Add caller. CallbacksDescriptor d(HandleName(Name::cast(caller-name())), - caller, ro_attribs); + caller, attribs); map-AppendDescriptor(d); } if (prototypeMode != DONT_ADD_PROTOTYPE) { if (prototypeMode == ADD_WRITEABLE_PROTOTYPE) { - ro_attribs = static_castPropertyAttributes(ro_attribs ~READ_ONLY); + attribs = static_castPropertyAttributes(attribs ~READ_ONLY); } HandleAccessorInfo prototype = -Accessors::FunctionPrototypeInfo(isolate(), ro_attribs); +Accessors::FunctionPrototypeInfo(isolate(), attribs); CallbacksDescriptor d(HandleName(Name::cast(prototype-name())), - prototype, ro_attribs); + prototype, attribs); map-AppendDescriptor(d); } } @@ -535,8 +533,6 @@ void Genesis::SetStrictFunctionInstanceDescriptor( static_castPropertyAttributes(DONT_ENUM | DONT_DELETE); PropertyAttributes ro_attribs = static_castPropertyAttributes(DONT_ENUM | DONT_DELETE | READ_ONLY); - PropertyAttributes roc_attribs = - static_castPropertyAttributes(DONT_ENUM | READ_ONLY); HandleAccessorInfo length = Accessors::FunctionLengthInfo(isolate(), ro_attribs); @@ -546,10 +542,10 @@ void Genesis::SetStrictFunctionInstanceDescriptor( map-AppendDescriptor(d); } HandleAccessorInfo name = - Accessors::FunctionNameInfo(isolate(), roc_attribs); + Accessors::FunctionNameInfo(isolate(), ro_attribs); { // Add name. CallbacksDescriptor d(HandleName(Name::cast(name-name())), - name, roc_attribs); + name, ro_attribs); map-AppendDescriptor(d); } { // Add arguments. Index: test/mjsunit/es7/object-observe.js diff --git a/test/mjsunit/es7/object-observe.js b/test/mjsunit/es7/object-observe.js index 20a2d160514ee554a9a091512d309a43a9ccd32a..7bb579f0c1462b6b33ff1bc6c3229067317f2845 100644 --- a/test/mjsunit/es7/object-observe.js +++ b/test/mjsunit/es7/object-observe.js @@ -1142,8 +1142,7 @@ var properties = [a, 1, 1, length, setPrototype, name, caller]; function blacklisted(obj, prop) { return (obj instanceof Int32Array prop == 1) || (obj instanceof Int32Array prop === length) || - (obj instanceof ArrayBuffer prop == 1) || - (obj instanceof Function
[v8-dev] [v8] r21632 committed - Revert Reland Make 'name' property on functions configurable....
Revision: 21632 Author: mvstan...@chromium.org Date: Tue Jun 3 11:52:07 2014 UTC Log: Revert Reland Make 'name' property on functions configurable. This reverts commit r21609 due to browser test failures. TBR=mstarzin...@chromium.org Review URL: https://codereview.chromium.org/313583002 http://code.google.com/p/v8/source/detail?r=21632 Modified: /branches/bleeding_edge/src/bootstrapper.cc /branches/bleeding_edge/test/mjsunit/es7/object-observe.js /branches/bleeding_edge/test/mjsunit/regress/regress-1530.js /branches/bleeding_edge/test/mjsunit/regress/regress-270142.js === --- /branches/bleeding_edge/src/bootstrapper.cc Tue Jun 3 08:12:43 2014 UTC +++ /branches/bleeding_edge/src/bootstrapper.cc Tue Jun 3 11:52:07 2014 UTC @@ -386,47 +386,45 @@ int size = (prototypeMode == DONT_ADD_PROTOTYPE) ? 4 : 5; Map::EnsureDescriptorSlack(map, size); - PropertyAttributes ro_attribs = - static_castPropertyAttributes(DONT_ENUM | DONT_DELETE | READ_ONLY); - PropertyAttributes roc_attribs = - static_castPropertyAttributes(DONT_ENUM | READ_ONLY); + PropertyAttributes attribs = static_castPropertyAttributes( + DONT_ENUM | DONT_DELETE | READ_ONLY); HandleAccessorInfo length = - Accessors::FunctionLengthInfo(isolate(), ro_attribs); + Accessors::FunctionLengthInfo(isolate(), attribs); { // Add length. CallbacksDescriptor d(HandleName(Name::cast(length-name())), - length, ro_attribs); + length, attribs); map-AppendDescriptor(d); } HandleAccessorInfo name = - Accessors::FunctionNameInfo(isolate(), roc_attribs); + Accessors::FunctionNameInfo(isolate(), attribs); { // Add name. CallbacksDescriptor d(HandleName(Name::cast(name-name())), - name, roc_attribs); + name, attribs); map-AppendDescriptor(d); } HandleAccessorInfo args = - Accessors::FunctionArgumentsInfo(isolate(), ro_attribs); + Accessors::FunctionArgumentsInfo(isolate(), attribs); { // Add arguments. CallbacksDescriptor d(HandleName(Name::cast(args-name())), - args, ro_attribs); + args, attribs); map-AppendDescriptor(d); } HandleAccessorInfo caller = - Accessors::FunctionCallerInfo(isolate(), ro_attribs); + Accessors::FunctionCallerInfo(isolate(), attribs); { // Add caller. CallbacksDescriptor d(HandleName(Name::cast(caller-name())), - caller, ro_attribs); + caller, attribs); map-AppendDescriptor(d); } if (prototypeMode != DONT_ADD_PROTOTYPE) { if (prototypeMode == ADD_WRITEABLE_PROTOTYPE) { - ro_attribs = static_castPropertyAttributes(ro_attribs ~READ_ONLY); + attribs = static_castPropertyAttributes(attribs ~READ_ONLY); } HandleAccessorInfo prototype = -Accessors::FunctionPrototypeInfo(isolate(), ro_attribs); +Accessors::FunctionPrototypeInfo(isolate(), attribs); CallbacksDescriptor d(HandleName(Name::cast(prototype-name())), - prototype, ro_attribs); + prototype, attribs); map-AppendDescriptor(d); } } @@ -535,8 +533,6 @@ static_castPropertyAttributes(DONT_ENUM | DONT_DELETE); PropertyAttributes ro_attribs = static_castPropertyAttributes(DONT_ENUM | DONT_DELETE | READ_ONLY); - PropertyAttributes roc_attribs = - static_castPropertyAttributes(DONT_ENUM | READ_ONLY); HandleAccessorInfo length = Accessors::FunctionLengthInfo(isolate(), ro_attribs); @@ -546,10 +542,10 @@ map-AppendDescriptor(d); } HandleAccessorInfo name = - Accessors::FunctionNameInfo(isolate(), roc_attribs); + Accessors::FunctionNameInfo(isolate(), ro_attribs); { // Add name. CallbacksDescriptor d(HandleName(Name::cast(name-name())), - name, roc_attribs); + name, ro_attribs); map-AppendDescriptor(d); } { // Add arguments. === --- /branches/bleeding_edge/test/mjsunit/es7/object-observe.js Mon Jun 2 13:35:26 2014 UTC +++ /branches/bleeding_edge/test/mjsunit/es7/object-observe.js Tue Jun 3 11:52:07 2014 UTC @@ -1142,8 +1142,7 @@ function blacklisted(obj, prop) { return (obj instanceof Int32Array prop == 1) || (obj instanceof Int32Array prop === length) || - (obj instanceof ArrayBuffer prop == 1) || - (obj instanceof Function prop === name) + (obj instanceof ArrayBuffer prop == 1) } for (var i in objects) for (var j in properties) { === --- /branches/bleeding_edge/test/mjsunit/regress/regress-1530.js Mon Jun 2 13:35:26 2014 UTC +++ /branches/bleeding_edge/test/mjsunit/regress/regress-1530.js Tue Jun 3 11:52:07 2014 UTC @@ -62,9 +62,8 @@
[v8-dev] Re: Revert Reland Make 'name' property on functions configurable. (issue 313583002)
Hi Michael, here is the revert, thx for the help, will let you know. https://codereview.chromium.org/313583002/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from it, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] Deopt maybe tenure allocation sites when semi-space is maximum size. (issue 307363002)
Good catch, lgtm. https://codereview.chromium.org/307363002/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from it, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] Version 3.27.18.1 (merged r21632) (issue 310773004)
Reviewers: mvstanton, Description: Version 3.27.18.1 (merged r21632) Revert Reland Make 'name' property on functions configurable. R=mvstan...@chromium.org BUG= Please review this at https://codereview.chromium.org/310773004/ SVN Base: https://v8.googlecode.com/svn/trunk Affected files (+20, -26 lines): M src/bootstrapper.cc M src/version.cc M test/mjsunit/es7/object-observe.js M test/mjsunit/regress/regress-1530.js M test/mjsunit/regress/regress-270142.js Index: src/bootstrapper.cc diff --git a/src/bootstrapper.cc b/src/bootstrapper.cc index 3a34182c41f65befb53ad70e8516235d043569ac..4c8701a55fddd8b522b299de7604c07fb5728cb8 100644 --- a/src/bootstrapper.cc +++ b/src/bootstrapper.cc @@ -386,47 +386,45 @@ void Genesis::SetFunctionInstanceDescriptor( int size = (prototypeMode == DONT_ADD_PROTOTYPE) ? 4 : 5; Map::EnsureDescriptorSlack(map, size); - PropertyAttributes ro_attribs = - static_castPropertyAttributes(DONT_ENUM | DONT_DELETE | READ_ONLY); - PropertyAttributes roc_attribs = - static_castPropertyAttributes(DONT_ENUM | READ_ONLY); + PropertyAttributes attribs = static_castPropertyAttributes( + DONT_ENUM | DONT_DELETE | READ_ONLY); HandleAccessorInfo length = - Accessors::FunctionLengthInfo(isolate(), ro_attribs); + Accessors::FunctionLengthInfo(isolate(), attribs); { // Add length. CallbacksDescriptor d(HandleName(Name::cast(length-name())), - length, ro_attribs); + length, attribs); map-AppendDescriptor(d); } HandleAccessorInfo name = - Accessors::FunctionNameInfo(isolate(), roc_attribs); + Accessors::FunctionNameInfo(isolate(), attribs); { // Add name. CallbacksDescriptor d(HandleName(Name::cast(name-name())), - name, roc_attribs); + name, attribs); map-AppendDescriptor(d); } HandleAccessorInfo args = - Accessors::FunctionArgumentsInfo(isolate(), ro_attribs); + Accessors::FunctionArgumentsInfo(isolate(), attribs); { // Add arguments. CallbacksDescriptor d(HandleName(Name::cast(args-name())), - args, ro_attribs); + args, attribs); map-AppendDescriptor(d); } HandleAccessorInfo caller = - Accessors::FunctionCallerInfo(isolate(), ro_attribs); + Accessors::FunctionCallerInfo(isolate(), attribs); { // Add caller. CallbacksDescriptor d(HandleName(Name::cast(caller-name())), - caller, ro_attribs); + caller, attribs); map-AppendDescriptor(d); } if (prototypeMode != DONT_ADD_PROTOTYPE) { if (prototypeMode == ADD_WRITEABLE_PROTOTYPE) { - ro_attribs = static_castPropertyAttributes(ro_attribs ~READ_ONLY); + attribs = static_castPropertyAttributes(attribs ~READ_ONLY); } HandleAccessorInfo prototype = -Accessors::FunctionPrototypeInfo(isolate(), ro_attribs); +Accessors::FunctionPrototypeInfo(isolate(), attribs); CallbacksDescriptor d(HandleName(Name::cast(prototype-name())), - prototype, ro_attribs); + prototype, attribs); map-AppendDescriptor(d); } } @@ -535,8 +533,6 @@ void Genesis::SetStrictFunctionInstanceDescriptor( static_castPropertyAttributes(DONT_ENUM | DONT_DELETE); PropertyAttributes ro_attribs = static_castPropertyAttributes(DONT_ENUM | DONT_DELETE | READ_ONLY); - PropertyAttributes roc_attribs = - static_castPropertyAttributes(DONT_ENUM | READ_ONLY); HandleAccessorInfo length = Accessors::FunctionLengthInfo(isolate(), ro_attribs); @@ -546,10 +542,10 @@ void Genesis::SetStrictFunctionInstanceDescriptor( map-AppendDescriptor(d); } HandleAccessorInfo name = - Accessors::FunctionNameInfo(isolate(), roc_attribs); + Accessors::FunctionNameInfo(isolate(), ro_attribs); { // Add name. CallbacksDescriptor d(HandleName(Name::cast(name-name())), - name, roc_attribs); + name, ro_attribs); map-AppendDescriptor(d); } { // Add arguments. Index: src/version.cc diff --git a/src/version.cc b/src/version.cc index 14dcef32b3fd6124e233b06eaef0eaf3d4c84b7f..e6a9c94b4960dc30a9396bbedeeec5cfaceed565 100644 --- a/src/version.cc +++ b/src/version.cc @@ -35,7 +35,7 @@ #define MAJOR_VERSION 3 #define MINOR_VERSION 27 #define BUILD_NUMBER 18 -#define PATCH_LEVEL 0 +#define PATCH_LEVEL 1 // Use 1 for candidates and 0 otherwise. // (Boolean macro values are not supported by all preprocessors.) #define IS_CANDIDATE_VERSION 0 Index: test/mjsunit/es7/object-observe.js diff --git a/test/mjsunit/es7/object-observe.js b/test/mjsunit/es7/object-observe.js index 20a2d160514ee554a9a091512d309a43a9ccd32a..7bb579f0c1462b6b33ff1bc6c3229067317f2845 100644 --- a/test/mjsunit/es7/object-observe.js +++
[v8-dev] Re: Version 3.27.18.1 (merged r21632) (issue 310773004)
lgtm https://codereview.chromium.org/310773004/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from it, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] Re: Version 3.27.18.1 (merged r21632) (issue 310773004)
Committed patchset #1 manually as r21633 (tree was closed). https://codereview.chromium.org/310773004/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from it, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] [v8] r21634 committed - Tagging version 3.27.18.1
Revision: 21634 Author: machenb...@chromium.org Date: Tue Jun 3 11:58:05 2014 UTC Log: Tagging version 3.27.18.1 http://code.google.com/p/v8/source/detail?r=21634 Added: /tags/3.27.18.1 -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from it, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] Re: Update include paths for gcmole (issue 310733005)
Right on, lgtm too. https://codereview.chromium.org/310733005/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from it, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] [v8] r21633 committed - Version 3.27.18.1 (merged r21632)...
Revision: 21633 Author: machenb...@chromium.org Date: Tue Jun 3 11:57:48 2014 UTC Log: Version 3.27.18.1 (merged r21632) Revert Reland Make 'name' property on functions configurable. R=mvstan...@chromium.org BUG= Review URL: https://codereview.chromium.org/310773004 http://code.google.com/p/v8/source/detail?r=21633 Modified: /trunk/src/bootstrapper.cc /trunk/src/version.cc /trunk/test/mjsunit/es7/object-observe.js /trunk/test/mjsunit/regress/regress-1530.js /trunk/test/mjsunit/regress/regress-270142.js === --- /trunk/src/bootstrapper.cc Tue Jun 3 00:04:55 2014 UTC +++ /trunk/src/bootstrapper.cc Tue Jun 3 11:57:48 2014 UTC @@ -386,47 +386,45 @@ int size = (prototypeMode == DONT_ADD_PROTOTYPE) ? 4 : 5; Map::EnsureDescriptorSlack(map, size); - PropertyAttributes ro_attribs = - static_castPropertyAttributes(DONT_ENUM | DONT_DELETE | READ_ONLY); - PropertyAttributes roc_attribs = - static_castPropertyAttributes(DONT_ENUM | READ_ONLY); + PropertyAttributes attribs = static_castPropertyAttributes( + DONT_ENUM | DONT_DELETE | READ_ONLY); HandleAccessorInfo length = - Accessors::FunctionLengthInfo(isolate(), ro_attribs); + Accessors::FunctionLengthInfo(isolate(), attribs); { // Add length. CallbacksDescriptor d(HandleName(Name::cast(length-name())), - length, ro_attribs); + length, attribs); map-AppendDescriptor(d); } HandleAccessorInfo name = - Accessors::FunctionNameInfo(isolate(), roc_attribs); + Accessors::FunctionNameInfo(isolate(), attribs); { // Add name. CallbacksDescriptor d(HandleName(Name::cast(name-name())), - name, roc_attribs); + name, attribs); map-AppendDescriptor(d); } HandleAccessorInfo args = - Accessors::FunctionArgumentsInfo(isolate(), ro_attribs); + Accessors::FunctionArgumentsInfo(isolate(), attribs); { // Add arguments. CallbacksDescriptor d(HandleName(Name::cast(args-name())), - args, ro_attribs); + args, attribs); map-AppendDescriptor(d); } HandleAccessorInfo caller = - Accessors::FunctionCallerInfo(isolate(), ro_attribs); + Accessors::FunctionCallerInfo(isolate(), attribs); { // Add caller. CallbacksDescriptor d(HandleName(Name::cast(caller-name())), - caller, ro_attribs); + caller, attribs); map-AppendDescriptor(d); } if (prototypeMode != DONT_ADD_PROTOTYPE) { if (prototypeMode == ADD_WRITEABLE_PROTOTYPE) { - ro_attribs = static_castPropertyAttributes(ro_attribs ~READ_ONLY); + attribs = static_castPropertyAttributes(attribs ~READ_ONLY); } HandleAccessorInfo prototype = -Accessors::FunctionPrototypeInfo(isolate(), ro_attribs); +Accessors::FunctionPrototypeInfo(isolate(), attribs); CallbacksDescriptor d(HandleName(Name::cast(prototype-name())), - prototype, ro_attribs); + prototype, attribs); map-AppendDescriptor(d); } } @@ -535,8 +533,6 @@ static_castPropertyAttributes(DONT_ENUM | DONT_DELETE); PropertyAttributes ro_attribs = static_castPropertyAttributes(DONT_ENUM | DONT_DELETE | READ_ONLY); - PropertyAttributes roc_attribs = - static_castPropertyAttributes(DONT_ENUM | READ_ONLY); HandleAccessorInfo length = Accessors::FunctionLengthInfo(isolate(), ro_attribs); @@ -546,10 +542,10 @@ map-AppendDescriptor(d); } HandleAccessorInfo name = - Accessors::FunctionNameInfo(isolate(), roc_attribs); + Accessors::FunctionNameInfo(isolate(), ro_attribs); { // Add name. CallbacksDescriptor d(HandleName(Name::cast(name-name())), - name, roc_attribs); + name, ro_attribs); map-AppendDescriptor(d); } { // Add arguments. === --- /trunk/src/version.cc Tue Jun 3 00:04:55 2014 UTC +++ /trunk/src/version.cc Tue Jun 3 11:57:48 2014 UTC @@ -35,7 +35,7 @@ #define MAJOR_VERSION 3 #define MINOR_VERSION 27 #define BUILD_NUMBER 18 -#define PATCH_LEVEL 0 +#define PATCH_LEVEL 1 // Use 1 for candidates and 0 otherwise. // (Boolean macro values are not supported by all preprocessors.) #define IS_CANDIDATE_VERSION 0 === --- /trunk/test/mjsunit/es7/object-observe.js Tue Jun 3 00:04:55 2014 UTC +++ /trunk/test/mjsunit/es7/object-observe.js Tue Jun 3 11:57:48 2014 UTC @@ -1142,8 +1142,7 @@ function blacklisted(obj, prop) { return (obj instanceof Int32Array prop == 1) || (obj instanceof Int32Array prop === length) || - (obj instanceof ArrayBuffer prop == 1) || - (obj instanceof Function prop === name) + (obj instanceof
[v8-dev] Re: Deopt maybe tenure allocation sites when semi-space is maximum size. (issue 307363002)
Reviewers: mvstanton, Message: Committed patchset #2 manually as r21635 (tree was closed). Description: Deopt maybe tenure allocation sites when semi-space is maximum size. BUG= R=mvstan...@chromium.org Committed: https://code.google.com/p/v8/source/detail?r=21635 Please review this at https://codereview.chromium.org/307363002/ SVN Base: https://v8.googlecode.com/svn/branches/bleeding_edge Affected files (+16, -3 lines): M src/heap.h M src/heap.cc M src/objects.h Index: src/heap.cc diff --git a/src/heap.cc b/src/heap.cc index ec058828e664cec507581ffe40db26b9edcbe2a7..2b8bc562b0c3881148a2459abc5cbd622f694674 100644 --- a/src/heap.cc +++ b/src/heap.cc @@ -497,10 +497,10 @@ void Heap::ProcessPretenuringFeedback() { // we grew to the maximum semi-space size to deopt maybe tenured // allocation sites. We could hold the maybe tenured allocation sites // in a seperate data structure if this is a performance problem. +bool deopt_maybe_tenured = DeoptMaybeTenuredAllocationSites(); bool use_scratchpad = -allocation_sites_scratchpad_length_ kAllocationSiteScratchpadSize -new_space_.IsAtMaximumCapacity() -maximum_size_scavenges_ == 0; + allocation_sites_scratchpad_length_ kAllocationSiteScratchpadSize + !deopt_maybe_tenured; int i = 0; Object* list_element = allocation_sites_list(); @@ -526,6 +526,11 @@ void Heap::ProcessPretenuringFeedback() { allocation_sites++; } + if (deopt_maybe_tenured site-IsMaybeTenure()) { +site-set_deopt_dependent_code(true); +trigger_deoptimization = true; + } + if (use_scratchpad) { i++; } else { Index: src/heap.h diff --git a/src/heap.h b/src/heap.h index b465ca47c485bb7223cd3f6713dc230258b000fe..bdc7fe11ef1ceda345d21057f0e21bdab2c3e6ca 100644 --- a/src/heap.h +++ b/src/heap.h @@ -1363,6 +1363,10 @@ class Heap { return maximum_size_scavenges_ 0; } + bool DeoptMaybeTenuredAllocationSites() { +return new_space_.IsAtMaximumCapacity() maximum_size_scavenges_ == 0; + } + // ObjectStats are kept in two arrays, counts and sizes. Related stats are // stored in a contiguous linear buffer. Stats groups are stored one after // another. Index: src/objects.h diff --git a/src/objects.h b/src/objects.h index 787e7ea87123b25fbf7fa74337c1ac4109eda209..ca8038f8938481046aaa4f2f0cb9eeaf9ac6c561 100644 --- a/src/objects.h +++ b/src/objects.h @@ -8503,6 +8503,10 @@ class AllocationSite: public Struct { return pretenure_decision() == kZombie; } + bool IsMaybeTenure() { +return pretenure_decision() == kMaybeTenure; + } + inline void MarkZombie(); inline bool MakePretenureDecision(PretenureDecision current_decision, -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from it, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] [v8] r21635 committed - Deopt maybe tenure allocation sites when semi-space is maximum size....
Revision: 21635 Author: hpa...@chromium.org Date: Tue Jun 3 11:59:47 2014 UTC Log: Deopt maybe tenure allocation sites when semi-space is maximum size. BUG= R=mvstan...@chromium.org Review URL: https://codereview.chromium.org/307363002 http://code.google.com/p/v8/source/detail?r=21635 Modified: /branches/bleeding_edge/src/heap.cc /branches/bleeding_edge/src/heap.h /branches/bleeding_edge/src/objects.h === --- /branches/bleeding_edge/src/heap.cc Tue Jun 3 08:28:38 2014 UTC +++ /branches/bleeding_edge/src/heap.cc Tue Jun 3 11:59:47 2014 UTC @@ -497,10 +497,10 @@ // we grew to the maximum semi-space size to deopt maybe tenured // allocation sites. We could hold the maybe tenured allocation sites // in a seperate data structure if this is a performance problem. +bool deopt_maybe_tenured = DeoptMaybeTenuredAllocationSites(); bool use_scratchpad = -allocation_sites_scratchpad_length_ kAllocationSiteScratchpadSize -new_space_.IsAtMaximumCapacity() -maximum_size_scavenges_ == 0; + allocation_sites_scratchpad_length_ kAllocationSiteScratchpadSize + !deopt_maybe_tenured; int i = 0; Object* list_element = allocation_sites_list(); @@ -525,6 +525,11 @@ } allocation_sites++; } + + if (deopt_maybe_tenured site-IsMaybeTenure()) { +site-set_deopt_dependent_code(true); +trigger_deoptimization = true; + } if (use_scratchpad) { i++; === --- /branches/bleeding_edge/src/heap.h Tue Jun 3 08:28:38 2014 UTC +++ /branches/bleeding_edge/src/heap.h Tue Jun 3 11:59:47 2014 UTC @@ -1362,6 +1362,10 @@ bool MaximumSizeScavenge() { return maximum_size_scavenges_ 0; } + + bool DeoptMaybeTenuredAllocationSites() { +return new_space_.IsAtMaximumCapacity() maximum_size_scavenges_ == 0; + } // ObjectStats are kept in two arrays, counts and sizes. Related stats are // stored in a contiguous linear buffer. Stats groups are stored one after === --- /branches/bleeding_edge/src/objects.h Tue Jun 3 08:12:43 2014 UTC +++ /branches/bleeding_edge/src/objects.h Tue Jun 3 11:59:47 2014 UTC @@ -8502,6 +8502,10 @@ bool IsZombie() { return pretenure_decision() == kZombie; } + + bool IsMaybeTenure() { +return pretenure_decision() == kMaybeTenure; + } inline void MarkZombie(); -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from it, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] Re: Add dependency on buildtools repo (issue 308353002)
Committed patchset #1 manually as r21636 (presubmit successful). https://codereview.chromium.org/308353002/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from it, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] [v8] r21636 committed - Add dependency on buildtools repo...
Revision: 21636 Author: joc...@chromium.org Date: Tue Jun 3 12:01:50 2014 UTC Log: Add dependency on buildtools repo Currently, this adds checkdeps, in the future, this will be the place where clang-format and gn lives. BUG=none R=machenb...@chromium.org LOG=n Review URL: https://codereview.chromium.org/308353002 http://code.google.com/p/v8/source/detail?r=21636 Modified: /branches/bleeding_edge/.DEPS.git /branches/bleeding_edge/DEPS /branches/bleeding_edge/Makefile === --- /branches/bleeding_edge/.DEPS.git Wed Apr 23 12:28:43 2014 UTC +++ /branches/bleeding_edge/.DEPS.git Tue Jun 3 12:01:50 2014 UTC @@ -13,6 +13,8 @@ deps = { 'v8/build/gyp': Var('git_url') + '/external/gyp.git@a3e2a5caf24a1e0a45401e09ad131210bf16b852', +'v8/buildtools': +Var('git_url') + '/chromium/buildtools.git@83ed7189066fd9b4b9ea15ffc2d4ab6d2da62571', 'v8/third_party/icu': Var('git_url') + '/chromium/deps/icu46.git@7a1ec88f69e25b3efcf76196d07f7815255db025', } === --- /branches/bleeding_edge/DEPSWed May 14 09:29:50 2014 UTC +++ /branches/bleeding_edge/DEPSTue Jun 3 12:01:50 2014 UTC @@ -4,6 +4,8 @@ vars = { chromium_trunk: https://src.chromium.org/svn/trunk;, + + buildtools_revision: 83ed7189066fd9b4b9ea15ffc2d4ab6d2da62571, } deps = { @@ -13,6 +15,10 @@ v8/third_party/icu: Var(chromium_trunk) + /deps/third_party/icu46@258359, + + v8/buildtools: +https://chromium.googlesource.com/chromium/buildtools.git@; + +Var(buildtools_revision), } deps_os = { === --- /branches/bleeding_edge/MakefileFri May 23 16:37:27 2014 UTC +++ /branches/bleeding_edge/MakefileTue Jun 3 12:01:50 2014 UTC @@ -468,3 +468,8 @@ svn checkout --force \ https://src.chromium.org/chrome/trunk/deps/third_party/icu46 \ third_party/icu --revision 258359 + ( cd buildtools || \ + git clone https://chromium.googlesource.com/chromium/buildtools.git; \ + cd buildtools; \ + git fetch origin; \ + git checkout 83ed7189066fd9b4b9ea15ffc2d4ab6d2da62571 ) -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from it, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] Re: Visit encountered JSWeakCollection list during scavenging. (issue 310783003)
lgtm https://codereview.chromium.org/310783003/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from it, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] Re: Fix leak in debug mirror cache. (issue 296953005)
On Tue, Jun 3, 2014 at 3:30 PM, Yang Guo yang...@chromium.org wrote: The thing is, if the debugger is not active, entering the debugger loads a new debug context every time. We assume that the same debug context is reused as long as we have debug event listener set in v8 and this matches with what I see in debug.cc [1]. Also we compile DebuggerScript.js once when devtools window opens and keep a persistent handle on a global object from the debug context. [1] https://code.google.com/p/chromium/codesearch#chromium/src/v8/src/debug.ccq=GetDebugContextsq=package:chromiumtype=csl=3134 You would have to re-disable the mirror cache every time. Is that what you want? Since we reuse one debug context during give debug session we will have to set that flag only once when opening devtools. Of cause if the user closes/reopens devtools a new debug context will be created and the flag will be reset. Yang On Tue, Jun 3, 2014 at 1:22 PM, Yury Semikhatsky yu...@chromium.org wrote: On Tue, Jun 3, 2014 at 1:37 PM, Yang Guo yang...@chromium.org wrote: So, could we simply skip the cache if the debugger is not entered? I would then set a flag on the global object of the debug context after entering the debugger, which guards caching. Leaving the debugger would clear the cache and clear the flag. We don't need to tie this to the debugger entrance, we just need a global flag for disabling mirror cache. We would set that flag in DebuggerScript.js initialization code. Since blink never enters the debugger, the mirror cache is not active. I would then remove the ClearMirrorCache call when getting scripts. Blink does enter the debugger in some cases, e.g. when doing evals on pause but we still don't need the cache in those cases. WDYT? Yang On Tue, Jun 3, 2014 at 11:29 AM, yu...@chromium.org wrote: On 2014/06/03 08:12:46, Yang wrote: https://codereview.chromium.org/296953005/diff/40001/src/ debug-debugger.js File src/debug-debugger.js (right): https://codereview.chromium.org/296953005/diff/40001/src/ debug-debugger.js#newcode491 src/debug-debugger.js:491: return %DebugGetLoadedScripts(); On 2014/06/03 08:07:21, yurys wrote: This change looks wrong to me. By design of V8 debugging protocol mirror cache should be kept while we are staying on a breakpoint so that the client could access corresponding object by its mirror id. After this change, however, the cache will be cleared on any call to GetLoadedScripts() and the remote id will become invalid. In case of blink the problem is that we create mirror objects not only when we stay on a breakpoint and I'm not sure we clear them properly. Also we don't use id-mirror map in blink and I believe a right way to fix this would be to disable mirror cache in blink completely. WDYT? Not sure I understand. Which code path does blink use to get loaded scripts? Blink adds a bunch of convenience methods into the debug context all of which are defined in DebuggerScript.js [1] and they leverage mirror infrastructure to inspected user objects (one important difference compared to v8 debugger as I said is that we never need to resolve id into morror object so the cache is useless in case of Blink). To collect current scripts PageScriptDebugServer.cpp calls [2] DebuggerScript.getScripts() which in turn calls Debug.scripts() [3] but all of this happens without entering debugger as simple function call using v8 api. As we don't enter debugger in that case ClearMirrorCache may never be called. This might be a bug in this particular case and we should use v8::Debug::Call for invoking DebuggerScript.getScripts() to make sure EnterDebugger is called and mirror cache is cleared after the call. But in general to avoid bugs like this we could skip mirror cache for the mirrors created by Blink. [1] https://code.google.com/p/chromium/codesearch#chromium/ src/third_party/WebKit/Source/bindings/v8/DebuggerScript.js [2] https://code.google.com/p/chromium/codesearch#chromium/ src/third_party/WebKit/Source/bindings/v8/PageScriptDebugServer.cppq= PageScriptDebugServer.cppsq=package:chromiumtype=csl=132 [3] https://code.google.com/p/chromium/codesearch#chromium/ src/third_party/WebKit/Source/bindings/v8/DebuggerScript.js q=getScriptssq=package:chromiumtype=csl=132 https://codereview.chromium.org/296953005/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from it, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] Enable pretenure call new. (issue 312723004)
Looks good, to me! okay, lgtm. https://codereview.chromium.org/312723004/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from it, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] Clusterfuzz identified overflow check needed in dehoisting. (issue 315593002)
Reviewers: danno, Message: Hi Danno, here is the fix we discussed. Perhaps the overflow check is overkill in the HStoreKeyed case? PTAL, --Michael Description: Clusterfuzz identified overflow check needed in dehoisting. BUG=380092 R=da...@chromium.org LOG=N Please review this at https://codereview.chromium.org/315593002/ SVN Base: https://v8.googlecode.com/svn/branches/bleeding_edge Affected files (+28, -25 lines): M src/hydrogen-dehoist.cc M src/hydrogen-instructions.h A + test/mjsunit/regress/regress-380092.js Index: src/hydrogen-dehoist.cc diff --git a/src/hydrogen-dehoist.cc b/src/hydrogen-dehoist.cc index e8008231ad3f22feaba64fbe9220bcd6c656a0ae..859b9eca8f39052be325de2467c90c8573786aa1 100644 --- a/src/hydrogen-dehoist.cc +++ b/src/hydrogen-dehoist.cc @@ -28,15 +28,17 @@ static void DehoistArrayIndex(ArrayInstructionInterface* array_operation) { if (!constant-HasInteger32Value()) return; int32_t sign = binary_operation-IsSub() ? -1 : 1; int32_t value = constant-Integer32Value() * sign; - // We limit offset values to 30 bits because we want to avoid the risk of - // overflows when the offset is added to the object header size. - if (value = 1 array_operation-MaxBaseOffsetBits() || value 0) return; + if (value 0) return; + uint32_t uvalue = static_castuint32_t(value) + ElementsKindToShiftSize(array_operation-elements_kind()); + // Protect against overflows when the offset is added to the object header + // size. + if (!array_operation-CanIncreaseBaseOffset(uvalue)) return; array_operation-SetKey(subexpression); if (binary_operation-HasNoUses()) { binary_operation-DeleteAndReplaceWith(NULL); } - value = ElementsKindToShiftSize(array_operation-elements_kind()); - array_operation-IncreaseBaseOffset(static_castuint32_t(value)); + array_operation-IncreaseBaseOffset(uvalue); array_operation-SetDehoisted(true); } Index: src/hydrogen-instructions.h diff --git a/src/hydrogen-instructions.h b/src/hydrogen-instructions.h index 05b31162af5730e4b83bb5844ed1aad40615f823..d106084a86c3f939267f02f4595ce26ca21f51f4 100644 --- a/src/hydrogen-instructions.h +++ b/src/hydrogen-instructions.h @@ -6426,8 +6426,8 @@ class ArrayInstructionInterface { virtual HValue* GetKey() = 0; virtual void SetKey(HValue* key) = 0; virtual ElementsKind elements_kind() const = 0; - virtual void IncreaseBaseOffset(uint32_t base_offset) = 0; - virtual int MaxBaseOffsetBits() = 0; + virtual bool CanIncreaseBaseOffset(uint32_t increase_by_value) = 0; + virtual void IncreaseBaseOffset(uint32_t increase_by_value) = 0; virtual bool IsDehoisted() = 0; virtual void SetDehoisted(bool is_dehoisted) = 0; virtual ~ArrayInstructionInterface() { } @@ -6474,12 +6474,13 @@ class HLoadKeyed V8_FINAL } bool HasDependency() const { return OperandAt(0) != OperandAt(2); } uint32_t base_offset() { return BaseOffsetField::decode(bit_field_); } - void IncreaseBaseOffset(uint32_t base_offset) { -base_offset += BaseOffsetField::decode(bit_field_); -bit_field_ = BaseOffsetField::update(bit_field_, base_offset); + bool CanIncreaseBaseOffset(uint32_t increase_by_value) { +increase_by_value += BaseOffsetField::decode(bit_field_); +return BaseOffsetField::is_valid(increase_by_value); } - virtual int MaxBaseOffsetBits() { -return kBitsForBaseOffset; + void IncreaseBaseOffset(uint32_t increase_by_value) { +increase_by_value += BaseOffsetField::decode(bit_field_); +bit_field_ = BaseOffsetField::update(bit_field_, increase_by_value); } HValue* GetKey() { return key(); } void SetKey(HValue* key) { SetOperandAt(1, key); } @@ -6935,11 +6936,13 @@ class HStoreKeyed V8_FINAL StoreFieldOrKeyedMode store_mode() const { return store_mode_; } ElementsKind elements_kind() const { return elements_kind_; } uint32_t base_offset() { return base_offset_; } - void IncreaseBaseOffset(uint32_t base_offset) { -base_offset_ += base_offset; + bool CanIncreaseBaseOffset(uint32_t increase_by_value) { +// unsigned integer addition wraps around. +uint32_t result = increase_by_value + base_offset_; +return result = base_offset_ result = increase_by_value; } - virtual int MaxBaseOffsetBits() { -return 31 - ElementsKindToShiftSize(elements_kind_); + void IncreaseBaseOffset(uint32_t increase_by_value) { +base_offset_ += increase_by_value; } HValue* GetKey() { return key(); } void SetKey(HValue* key) { SetOperandAt(1, key); } Index: test/mjsunit/regress/regress-380092.js diff --git a/test/mjsunit/regress/regress-alloc-smi-check.js b/test/mjsunit/regress/regress-380092.js similarity index 56% copy from test/mjsunit/regress/regress-alloc-smi-check.js copy to test/mjsunit/regress/regress-380092.js index 295048a13ef862ceb21939de104e7968dd7772da..a7634355ca5d0db9a867cb691b05129b984911f8 100644 --- a/test/mjsunit/regress/regress-alloc-smi-check.js +++ b/test/mjsunit/regress/regress-380092.js @@
[v8-dev] Re: Add support for extended constant pool arrays. (issue 304143002)
Thanks for the review Ulan, PTAL. https://codereview.chromium.org/304143002/diff/20001/src/factory.cc File src/factory.cc (right): https://codereview.chromium.org/304143002/diff/20001/src/factory.cc#newcode134 src/factory.cc:134: HandleConstantPoolArray Factory::NewExtendedConstantPoolArray( On 2014/06/02 12:32:58, ulan wrote: Instead of passing this long list arguments around, what if we introduce a class NumberOfEntries that stores int number_of_entries[4]? The this function would look like: NewExtendedConstantPoolArray(NumberOfEntries small, NumberOfEntries extended); Constant pool would have a method: NumberOfEntries ConstantPoolArray::number_of_entries(Section section); This would also simplify SizeFor* methods. Good idea - done. https://codereview.chromium.org/304143002/diff/20001/src/objects-inl.h File src/objects-inl.h (right): https://codereview.chromium.org/304143002/diff/20001/src/objects-inl.h#newcode2237 src/objects-inl.h:2237: int index = 0; On 2014/06/02 12:32:58, ulan wrote: How about iterating over types and sections in this and other functions? Something like this (modulo needs enum casts): for (int s = 0; s = section; ++s) { for (int t = 0; t type; ++t) { index += number_of_entries(t, s); } } Done (but only iterating through types). https://codereview.chromium.org/304143002/diff/20001/src/objects-inl.h#newcode2317 src/objects-inl.h:2317: if (index = first_index(INT64, section) On 2014/06/02 12:32:58, ulan wrote: int t = 0; while (last_index(t, section) index) ++t; return t; Done. https://codereview.chromium.org/304143002/diff/20001/src/objects.cc File src/objects.cc (right): https://codereview.chromium.org/304143002/diff/20001/src/objects.cc#newcode9980 src/objects.cc:9980: if (number_of_entries(CODE_PTR, SMALL_SECTION) 0) { On 2014/06/02 12:32:58, ulan wrote: Type type[] = {CODE_PTR, HEAP_PTR}; Address value[] = {illegal builtin entry, undefined}; for (int section = 0; section = final_section(); ++section) { for (int i = 0; i 2; ++i) { if (number_of_entries(type[i], section) 0) { int offset = OffsetOfElementAt(first_index(type[i], section)); MemsetPointer( HeapObject::RawField(this, offset), value[i], number_of_entries(type[i], section)); } } } Done. I'm not convinced this is clearer though. https://codereview.chromium.org/304143002/diff/20001/src/objects.h File src/objects.h (right): https://codereview.chromium.org/304143002/diff/20001/src/objects.h#newcode3265 src/objects.h:3265: offset += kInt64Size * Min(Max(0, index - first_index(INT64, section)), On 2014/06/02 12:32:58, ulan wrote: int t = 0; while (last_index(t, section) index) { offset += entry_size(t) * number_of_entries(t, section); ++t; } offset += entry_size(type) * (index - first_index(t, section)); Done (somewhat differently both to avoid lots of static casts). https://codereview.chromium.org/304143002/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from it, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] Re: Add support for extended constant pool arrays. (issue 304143002)
On 2014/06/02 13:14:00, mvstanton wrote: Just as a DBC: this is great. I'm thinking to be able to use these handy ConstantPoolArrays as elements in the feedback vector when feedback for a particular node (say a LoadIC) would like to store code addresses as well as object pointers. You've solved a thorny problem and type vector code could piggy-back on that. Great, sounds good. Let me know if you would like to have a chat about this over VC, or in-person the week after next when I'm in Munich. https://codereview.chromium.org/304143002/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from it, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] Add DEPS files and run checkdeps in presubmit check (issue 312763002)
Reviewers: Jakob, Description: Add DEPS files and run checkdeps in presubmit check BUG=none R=jkumme...@chromium.org LOG=n Please review this at https://codereview.chromium.org/312763002/ SVN Base: https://v8.googlecode.com/svn/branches/bleeding_edge Affected files (+96, -0 lines): M DEPS M PRESUBMIT.py A src/DEPS A src/base/DEPS A src/libplatform/DEPS A test/cctest/DEPS A tools/DEPS Index: DEPS diff --git a/DEPS b/DEPS index 47cb78223422aaea4b2f144f76e3e37c7cec1d75..836e75350188511f8ef93a9815277f9fd3689777 100644 --- a/DEPS +++ b/DEPS @@ -31,6 +31,18 @@ deps_os = { } } +include_rules = [ + # Everybody can use some things. + +include, + +unicode, +] + +# checkdeps.py shouldn't check for includes in these directories: +skip_child_includes = [ + build, + third_party, +] + hooks = [ { # A change to a .gyp, .gypi, or to GYP itself should run the generator. Index: PRESUBMIT.py diff --git a/PRESUBMIT.py b/PRESUBMIT.py index 6a836e0fefbf25f672a05bc71398e541e3dc1eec..70f576a81a4de6498eb54c20020219b6cf8ca385 100644 --- a/PRESUBMIT.py +++ b/PRESUBMIT.py @@ -31,6 +31,9 @@ See http://dev.chromium.org/developers/how-tos/depottools/presubmit-scripts for more details about the presubmit API built into gcl. +import sys + + def _V8PresubmitChecks(input_api, output_api): Runs the V8 presubmit checks. import sys @@ -49,12 +52,66 @@ def _V8PresubmitChecks(input_api, output_api): return results +def _CheckUnwantedDependencies(input_api, output_api): + Runs checkdeps on #include statements added in this + change. Breaking - rules is an error, breaking ! rules is a + warning. + + # We need to wait until we have an input_api object and use this + # roundabout construct to import checkdeps because this file is + # eval-ed and thus doesn't have __file__. + original_sys_path = sys.path + try: +sys.path = sys.path + [input_api.os_path.join( +input_api.PresubmitLocalPath(), 'buildtools', 'checkdeps')] +import checkdeps +from cpp_checker import CppChecker +from rules import Rule + finally: +# Restore sys.path to what it was before. +sys.path = original_sys_path + + added_includes = [] + for f in input_api.AffectedFiles(): +if not CppChecker.IsCppFile(f.LocalPath()): + continue + +changed_lines = [line for line_num, line in f.ChangedContents()] +added_includes.append([f.LocalPath(), changed_lines]) + + deps_checker = checkdeps.DepsChecker(input_api.PresubmitLocalPath()) + + error_descriptions = [] + warning_descriptions = [] + for path, rule_type, rule_description in deps_checker.CheckAddedCppIncludes( + added_includes): +description_with_path = '%s\n%s' % (path, rule_description) +if rule_type == Rule.DISALLOW: + error_descriptions.append(description_with_path) +else: + warning_descriptions.append(description_with_path) + + results = [] + if error_descriptions: +results.append(output_api.PresubmitError( +'You added one or more #includes that violate checkdeps rules.', +error_descriptions)) + if warning_descriptions: +results.append(output_api.PresubmitPromptOrNotify( +'You added one or more #includes of files that are temporarily\n' +'allowed but being removed. Can you avoid introducing the\n' +'#include? See relevant DEPS file(s) for details and contacts.', +warning_descriptions)) + return results + + def _CommonChecks(input_api, output_api): Checks common to both upload and commit. results = [] results.extend(input_api.canned_checks.CheckOwners( input_api, output_api, source_file_filter=None)) results.extend(_V8PresubmitChecks(input_api, output_api)) + results.extend(_CheckUnwantedDependencies(input_api, output_api)) return results Index: src/DEPS diff --git a/src/DEPS b/src/DEPS new file mode 100644 index ..4196627416fc2830778d3233792c2c8c238a1e9d --- /dev/null +++ b/src/DEPS @@ -0,0 +1,6 @@ +include_rules = [ + +src, + + # TODO(jochen): Enable this. + #-src/libplatform, +] Index: src/base/DEPS diff --git a/src/base/DEPS b/src/base/DEPS new file mode 100644 index ..6548030243810577eb1459a4fff5f9b2db09a50c --- /dev/null +++ b/src/base/DEPS @@ -0,0 +1,4 @@ +include_rules = [ + -src, + +src/base, +] Index: src/libplatform/DEPS diff --git a/src/libplatform/DEPS b/src/libplatform/DEPS new file mode 100644 index ..bace5d31727fdbf50e16eacf87248abe1403f751 --- /dev/null +++ b/src/libplatform/DEPS @@ -0,0 +1,6 @@ +include_rules = [ + # TODO(jochen): Enable this. + #-src, + +src/base, + +src/libplatform, +] Index: test/cctest/DEPS diff --git a/test/cctest/DEPS b/test/cctest/DEPS new file mode 100644 index ..3e73aa244ff09076a1e05734d75bf0ca620a86cf --- /dev/null +++ b/test/cctest/DEPS @@ -0,0 +1,3 @@
[v8-dev] Re: Special case ConstantPoolArray in MarkCompactCollector::MigrateObject. (issue 304223002)
On 2014/05/29 15:54:12, rmcilroy wrote: Given that ConstantPoolArrays can contain raw int32 or int64 values, I think this change is necessary to ensure we don't confuse a raw int value as a tagged pointer when migrating a ConstantPoolArray. PTAL. Ping? Could you take a look please Michael? https://codereview.chromium.org/304223002/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from it, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.