Re: [v8-dev] Re: This feature implements IMM32 encoding for user defined values with a new ran... (issue3169050)

2010-08-27 Thread Lasse R.H. Nielsen
If the seed is random by default, but there is a command line to use a specific seed, and the seed is part of a crash-dump, we can debug using the exact same code as the one that crashed. It's the same thing we do with Math.random when running benchmarks - we use a random seed by default but can o

[v8-dev] Re: Cleanup the way the debugger stores live registers when entering at a break... (issue3141047)

2010-08-27 Thread sgjesse
http://codereview.chromium.org/3141047/diff/1/10 File src/debug.h (right): http://codereview.chromium.org/3141047/diff/1/10#newcode332 src/debug.h:332: k_after_break_target_address, On 2010/08/26 13:31:04, Kasper Lund wrote: Weird naming. Should all be kAfterBreak... right? Feel free to keep it

[v8-dev] Add file missing from last commit. (issue3248001)

2010-08-27 Thread sgjesse
Reviewers: Kasper Lund, Description: Add file missing from last commit. tbr=kasp...@chromium.org Please review this at http://codereview.chromium.org/3248001/show SVN Base: http://v8.googlecode.com/svn/branches/bleeding_edge/ Affected files: M src/globals.h Index: src/globals.h ==

[v8-dev] Re: Add file missing from last commit. (issue3248001)

2010-08-27 Thread kasperl
LGTM. http://codereview.chromium.org/3248001/show -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev

[v8-dev] Fix presumbit errors in previous commit (issue3249001)

2010-08-27 Thread sgjesse
Reviewers: Kasper Lund, Description: Fix presumbit errors in previous commit tbr=kasp...@chromium.org Please review this at http://codereview.chromium.org/3249001/show SVN Base: http://v8.googlecode.com/svn/branches/bleeding_edge/ Affected files: M src/debug.h M src/x64/debug-x64.

[v8-dev] Re: Fix presumbit errors in previous commit (issue3249001)

2010-08-27 Thread kasperl
Fantastic. LGTM. http://codereview.chromium.org/3249001/show -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev

[v8-dev] Re: Reordered function entries in PreParse data to be ordered by start position. (issue3185026)

2010-08-27 Thread lrn
Found bug. Please re-review. http://codereview.chromium.org/3185026/show -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev

[v8-dev] Re: Reordered function entries in PreParse data to be ordered by start position. (issue3185026)

2010-08-27 Thread ager
LGTM http://codereview.chromium.org/3185026/diff/9001/10001 File src/parser.cc (right): http://codereview.chromium.org/3185026/diff/9001/10001#newcode948 src/parser.cc:948: if (index_ + FunctionEntry::kSize <= store_.length() A couple of extra parenthesis would help me read this. :) http://cod

[v8-dev] Remove dependence of code-stubs on codegen, the virtual frame code generator.... (issue3169049)

2010-08-27 Thread Kevin Millikin
Forwarding, subscribed to v8-dev :) -- Forwarded message -- From: Date: Fri, Aug 27, 2010 at 10:27 AM Subject: Re: Remove dependence of code-stubs on codegen, the virtual frame code generator (issue3169049) To: whe...@chromium.org Cc: v8-dev@googlegroups.com My concern is ma

[v8-dev] Re: Using array index hash code for string-to-number conversion. (issue3141022)

2010-08-27 Thread vegorov
LGTM http://codereview.chromium.org/3141022/diff/28006/34008 File src/arm/macro-assembler-arm.cc (right): http://codereview.chromium.org/3141022/diff/28006/34008#newcode1340 src/arm/macro-assembler-arm.cc:1340: void MacroAssembler::IndexFromHash(Register key, Register hash) { makes sense to ren

[v8-dev] Re: Using array index hash code for string-to-number conversion. (issue3141022)

2010-08-27 Thread serya
http://codereview.chromium.org/3141022/diff/28006/34008 File src/arm/macro-assembler-arm.cc (right): http://codereview.chromium.org/3141022/diff/28006/34008#newcode1340 src/arm/macro-assembler-arm.cc:1340: void MacroAssembler::IndexFromHash(Register key, Register hash) { On 2010/08/27 08:57:59,

[v8-dev] Improve header file inclusions. Drop some unneeded includes, and add some ne... (issue3253001)

2010-08-27 Thread whesse
Reviewers: Kevin Millikin, Description: Improve header file inclusions. Drop some unneeded includes, and add some needed ones. Please review this at http://codereview.chromium.org/3253001/show SVN Base: http://v8.googlecode.com/svn/branches/bleeding_edge/ Affected files: M src/contexts.

[v8-dev] Re: Improve header file inclusions. Drop some unneeded includes, and add some ne... (issue3253001)

2010-08-27 Thread kmillikin
Nice. LGTM. http://codereview.chromium.org/3253001/show -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev

[v8-dev] Reverting r5362. (issue3217003)

2010-08-27 Thread serya
Reviewers: Vyacheslav Egorov, Message: To fix build error (missing files). Description: Reverting r5362. Please review this at http://codereview.chromium.org/3217003/show SVN Base: http://v8.googlecode.com/svn/branches/bleeding_edge/ Affected files: M src/arm/codegen-arm.h M src/a

[v8-dev] Re: Reverting r5362. (issue3217003)

2010-08-27 Thread vegorov
LGTM http://codereview.chromium.org/3217003/show -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev

[v8-dev] Fixing build error r5362 (adding missing files). (issue3258001)

2010-08-27 Thread serya
Reviewers: Vyacheslav Egorov, Description: Fixing build error r5362 (adding missing files). Please review this at http://codereview.chromium.org/3258001/show SVN Base: http://v8.googlecode.com/svn/branches/bleeding_edge/ Affected files: M src/objects.h M src/objects.cc Index: sr

[v8-dev] Re: Fixing build error r5362 (adding missing files). (issue3258001)

2010-08-27 Thread vegorov
LGTM http://codereview.chromium.org/3258001/show -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev

[v8-dev] Force relinking of paged space if first attempt to recommit from space fails. (issue3260001)

2010-08-27 Thread antonm
Reviewers: Vyacheslav Egorov, Message: Slava, may you have a look? Maybe newly added function should be rather named EnsureInChunkOrder. And maybe we should deallocate functions out of mark compactor and into spaces: after all it is a space who knows how to deallocate a stuff from it. Desc

[v8-dev] Add os_error stat to oom_dump utility. (issue3150028)

2010-08-27 Thread antonm
Reviewers: Mads Ager, Message: Mads, tiny review for you if you please. Description: Add os_error stat to oom_dump utility. Please review this at http://codereview.chromium.org/3150028/show Affected files: M tools/oom_dump/oom_dump.cc Index: tools/oom_dump/oom_dump.cc diff --git a/tools/o

[v8-dev] Check result of JSObject::NormalizeProperties() in JSObject::PreventExtension... (issue3262001)

2010-08-27 Thread vegorov
Reviewers: antonm, Description: Check result of JSObject::NormalizeProperties() in JSObject::PreventExtensions(). Normalization requires allocation so Failure object can be returned. BUG=http://code.google.com/p/v8/issues/detail?id=851 TEST=test/mjsunit/regress/regress-851.js Please review th

[v8-dev] Re: Check result of JSObject::NormalizeProperties() in JSObject::PreventExtension... (issue3262001)

2010-08-27 Thread antonm
LGTM! http://codereview.chromium.org/3262001/diff/1/3 File test/mjsunit/regress/regress-851.js (right): http://codereview.chromium.org/3262001/diff/1/3#newcode1 test/mjsunit/regress/regress-851.js:1: // Copyright 2008 the V8 project authors. All rights reserved. nit: 2008 -> 2010 http://codere

[v8-dev] Fix regress-851.js to use assertNull instead of assertFalse. (issue3232002)

2010-08-27 Thread vegorov
Reviewers: antonm, Description: Fix regress-851.js to use assertNull instead of assertFalse. Please review this at http://codereview.chromium.org/3232002/show SVN Base: http://v8.googlecode.com/svn/branches/bleeding_edge/ Affected files: M test/mjsunit/regress/regress-851.js Index: tes

[v8-dev] Re: Fix regress-851.js to use assertNull instead of assertFalse. (issue3232002)

2010-08-27 Thread Anton Muhin
LGTM On Fri, Aug 27, 2010 at 5:19 PM, wrote: > Reviewers: antonm, > > Description: > Fix regress-851.js to use assertNull instead of assertFalse. > > Please review this at http://codereview.chromium.org/3232002/show > > SVN Base: http://v8.googlecode.com/svn/branches/bleeding_edge/ > > Affected

[v8-dev] Re: Force relinking of paged space if first attempt to recommit from space fails. (issue3260001)

2010-08-27 Thread vegorov
LGTM with comment addressed. Thanks for cleaning debris left after me! http://codereview.chromium.org/3260001/diff/1/4 File src/mark-compact.h (right): http://codereview.chromium.org/3260001/diff/1/4#newcode122 src/mark-compact.h:122: static void DeallocateOldPointerBlock(Address start, Maybe

[v8-dev] Re: Force relinking of paged space if first attempt to recommit from space fails. (issue3260001)

2010-08-27 Thread vegorov
http://codereview.chromium.org/3260001/diff/1/2 File src/heap.cc (right): http://codereview.chromium.org/3260001/diff/1/2#newcode544 src/heap.cc:544: Heap::old_pointer_space()->RelinkPageListInChunkOrder( After turning block deallocation into virtual method. This might be turned into simple loop

Re: [v8-dev] Re: GCController.collect() not aggressive enough?

2010-08-27 Thread Ojan Vafai
Oh, interesting. Do you have a pointer to those tests? I'd like to take a look to see if they're doing something similar. I'm fully aware that this can't be made to work for 100% of cases, but my hope is that it will be sufficiently stable for a large subset. Ojan On Thu, Aug 26, 2010 at 11:15 P