[v8-dev] Re: Lower waste from alignment of deferred code blocks. (issue2809029)

2010-06-22 Thread erik . corry
On 2010/06/23 06:46:44, Erik Corry wrote: The idea of deferred code is that it normally doesn't get run, so lets just reduce this to zero to avoid wasting memory. At which point it will LGTM. http://codereview.chromium.org/2809029/show -- v8-dev mailing list v8-dev@googlegroups.com http://g

[v8-dev] Re: Lower waste from alignment of deferred code blocks. (issue2809029)

2010-06-22 Thread erik . corry
The idea of deferred code is that it normally doesn't get run, so lets just reduce this to zero to avoid wasting memory. http://codereview.chromium.org/2809029/show -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev

[v8-dev] [Isolates] Make statics from BootstrapperActive to be instance members of Boo... (issue2813026)

2010-06-22 Thread Maxim . Mossienko
Reviewers: Mads Ager, Vitaly, Dmitry Titov, zarko, Message: Please, review Description: [Isolates] Make statics from BootstrapperActive to be instance members of Bootstrapper. Bootstrapper::IsActive is also instance method now. Please review this at http://codereview.chromium.org/2813026/show

[v8-dev] [v8] r4919 committed - [Isolates] Use TLS for global isolate storage by default....

2010-06-22 Thread codesite-noreply
Revision: 4919 Author: lukezarko Date: Tue Jun 22 15:25:03 2010 Log: [Isolates] Use TLS for global isolate storage by default. Review URL: http://codereview.chromium.org/2876005 http://code.google.com/p/v8/source/detail?r=4919 Modified: /branches/experimental/isolates/src/isolate.h ===

[v8-dev] Re: Issue 494 in v8: New snapshot code breaks mjsunit/apply on mac debug with snapshots

2010-06-22 Thread codesite-noreply
Comment #1 on issue 494 by lukezarko: New snapshot code breaks mjsunit/apply on mac debug with snapshots http://code.google.com/p/v8/issues/detail?id=494 More in issue 742. -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev

[v8-dev] Re: Issue 742 in v8: Failing test (mjsunit/apply) in bleeding_edge r4897

2010-06-22 Thread codesite-noreply
Comment #6 on issue 742 by lukezarko: Failing test (mjsunit/apply) in bleeding_edge r4897 http://code.google.com/p/v8/issues/detail?id=742 Committed a change to the test: http://codereview.chromium.org/2835014/show -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group

[v8-dev] [v8] r4918 committed - Make the apply.js unit test more resilient to differing stack position...

2010-06-22 Thread codesite-noreply
Revision: 4918 Author: lukezarko Date: Tue Jun 22 15:20:58 2010 Log: Make the apply.js unit test more resilient to differing stack positions. More information is at http://code.google.com/p/v8/issues/detail?id=742 Review URL: http://codereview.chromium.org/2835014 http://code.google.com/p/v8/

[v8-dev] Re: [Isolates] Use TLS for global isolate storage by default. (issue2876005)

2010-06-22 Thread dimich
LGTM http://codereview.chromium.org/2876005/show -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev

[v8-dev] Re: Make the apply.js unit test more resilient to differing stack positions.... (issue2835014)

2010-06-22 Thread vitalyr
http://codereview.chromium.org/2835014/diff/5001/6002 File test/mjsunit/mjsunit.status (left): http://codereview.chromium.org/2835014/diff/5001/6002#oldcode39 test/mjsunit/mjsunit.status:39: On 2010/06/22 21:48:20, zarko wrote: Is this sufficient to re-enable the test? I think so, yes. After

[v8-dev] [Isolates] Use TLS for global isolate storage by default. (issue2876005)

2010-06-22 Thread lukezarko
Reviewers: Dmitry Titov, Message: From this morning's mail! Thanks Luke Description: [Isolates] Use TLS for global isolate storage by default. Please review this at http://codereview.chromium.org/2876005/show SVN Base: http://v8.googlecode.com/svn/branches/experimental/isolates/ Affected

[v8-dev] Re: Make the apply.js unit test more resilient to differing stack positions.... (issue2835014)

2010-06-22 Thread lukezarko
Just want to make sure that I'm doing this correctly. Thanks Luke http://codereview.chromium.org/2835014/diff/5001/6002 File test/mjsunit/mjsunit.status (left): http://codereview.chromium.org/2835014/diff/5001/6002#oldcode39 test/mjsunit/mjsunit.status:39: Is this sufficient to re-enable the

[v8-dev] [Isolates] Remove statics from malloc + dependencies. (issue2841023)

2010-06-22 Thread lukezarko
Reviewers: Vitaly, Dmitry Titov, Message: Thanks Luke http://codereview.chromium.org/2841023/diff/29001/30004 File src/api.h (left): http://codereview.chromium.org/2841023/diff/29001/30004#oldcode168 src/api.h:168: static RegisteredExtension* first_auto_extension_; This is not used. http://

[v8-dev] Re: Issue 746 in v8: Inline fast-pass MUL binary operation on ARM

2010-06-22 Thread codesite-noreply
Comment #2 on issue 746 by zha...@codeaurora.org: Inline fast-pass MUL binary operation on ARM http://code.google.com/p/v8/issues/detail?id=746 A draft version is uploaded here http://codereview.chromium.org/2868018/show. It's not ready for review yet. We're going to finalize it soon, aft

[v8-dev] Re: Issue 745 in v8: global/local eval forces v8 to crash if uses second eval call parameter

2010-06-22 Thread codesite-noreply
Updates: Status: PendingFurtherInfo Comment #2 on issue 745 by a...@chromium.org: global/local eval forces v8 to crash if uses second eval call parameter http://code.google.com/p/v8/issues/detail?id=745 I attempted to reproduce this and was unable to. Could you provide some more inf

[v8-dev] [v8] r4917 committed - Heap profiler: perform a GC round before taking a snapshot...

2010-06-22 Thread codesite-noreply
Revision: 4917 Author: mikhail.naga...@gmail.com Date: Tue Jun 22 07:58:08 2010 Log: Heap profiler: perform a GC round before taking a snapshot to get rid of global object loaded from a snapshot. This eliminates the "double global object" issue. Thanks to Mads for suggesting this! Review URL: htt

[v8-dev] Re: Heap profiler: perform a GC round before taking a snapshot (issue2865013)

2010-06-22 Thread ager
LGTM http://codereview.chromium.org/2865013/show -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev

[v8-dev] Make SampleRateCalculator test resilient to float precision issues. (issue2811021)

2010-06-22 Thread mnaganov
Reviewers: Søren Gjesse, Description: Make SampleRateCalculator test resilient to float precision issues. Tested with kSamplingIntervalMs values 2 and 4. Please review this at http://codereview.chromium.org/2811021/show Affected files: M test/cctest/test-profile-generator.cc Index: test/cc

[v8-dev] Heap profiler: perform a GC round before taking a snapshot (issue2865013)

2010-06-22 Thread mnaganov
Reviewers: Søren Gjesse, Mads Ager, Description: Heap profiler: perform a GC round before taking a snapshot to get rid of global object loaded from a snapshot. This eliminates the "double global object" issue. Thanks to Mads for suggesting this! Please review this at http://codereview.chromium.o

[v8-dev] Add "has fast elements" bit to maps and use it in inlined keyed loads. (issue2870018)

2010-06-22 Thread vitalyr
Reviewers: Mads Ager, Description: Add "has fast elements" bit to maps and use it in inlined keyed loads. A potential issue with this change is creating lots of maps when objects flip between fast/slow elements modes. We could add special transitions to avoid this. Yet testing this on our benc

[v8-dev] Re: Issue 642 in v8: Implement CallStubCompiler::CompileArrayPopCall for ARM

2010-06-22 Thread codesite-noreply
Updates: Summary: Implement CallStubCompiler::CompileArrayPopCall for ARM Comment #2 on issue 642 by lasserei...@gmail.com: Implement CallStubCompiler::CompileArrayPopCall for ARM http://code.google.com/p/v8/issues/detail?id=642 Fixed for X64 in revision 4579. -- v8-dev mailing list

[v8-dev] Re: Issue 639 in v8: Implement CallStubCompiler::CompileArrayPushCall for ARM

2010-06-22 Thread codesite-noreply
Updates: Summary: Implement CallStubCompiler::CompileArrayPushCall for ARM Comment #4 on issue 639 by lasserei...@gmail.com: Implement CallStubCompiler::CompileArrayPushCall for ARM http://code.google.com/p/v8/issues/detail?id=639 (No comment was entered for this change.) -- v8-dev m

[v8-dev] Re: Issue 639 in v8: Implement CallStubCompiler::CompileArrayPushCall for x64 and ARM

2010-06-22 Thread codesite-noreply
Comment #3 on issue 639 by lasserei...@gmail.com: Implement CallStubCompiler::CompileArrayPushCall for x64 and ARM http://code.google.com/p/v8/issues/detail?id=639 X64 version added in revision 4579. -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev

[v8-dev] Re: Issue 625 in v8: No IsUint32() in v8::Value

2010-06-22 Thread codesite-noreply
Updates: Status: Fixed Comment #3 on issue 625 by lasserei...@gmail.com: No IsUint32() in v8::Value http://code.google.com/p/v8/issues/detail?id=625 Added in revision 4008. -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev

[v8-dev] Re: Issue 731 in v8: scanner.h's class KeywordMatcher's constructor does not initialize all member variables

2010-06-22 Thread codesite-noreply
Updates: Status: Fixed Comment #2 on issue 731 by lasserei...@gmail.com: scanner.h's class KeywordMatcher's constructor does not initialize all member variables http://code.google.com/p/v8/issues/detail?id=731 Fixed in revision 4916 -- v8-dev mailing list v8-dev@googlegroups.com http

[v8-dev] Re: Make the apply.js unit test more resilient to differing stack positions.... (issue2835014)

2010-06-22 Thread vitalyr
LGTM test/mjsunit/mjsunit.status should be updated to enable the test. http://codereview.chromium.org/2835014/diff/1/2 File test/mjsunit/apply.js (right): http://codereview.chromium.org/2835014/diff/1/2#newcode115 test/mjsunit/apply.js:115: var stack_corner_case_failure = 0; Use boolean? http

[v8-dev] Lower waste from alignment of deferred code blocks. (issue2809029)

2010-06-22 Thread lrn
Reviewers: Erik Corry, Description: Lower waste from alignment of deferred code blocks. Some ARM chips load instructions 8 byte at a time. Please review this at http://codereview.chromium.org/2809029/show Affected files: M src/arm/assembler-arm.cc Index: src/arm/assembler-arm.cc diff --git

[v8-dev] [v8] r4916 committed - Made scanner follow coding style....

2010-06-22 Thread codesite-noreply
Revision: 4916 Author: l...@chromium.org Date: Tue Jun 22 05:31:24 2010 Log: Made scanner follow coding style. Review URL: http://codereview.chromium.org/2832018 http://code.google.com/p/v8/source/detail?r=4916 Modified: /branches/bleeding_edge/src/scanner.cc /branches/bleeding_edge/src/scanne

[v8-dev] Re: Made scanner follow coding style. (issue2832018)

2010-06-22 Thread ricow
LGTM http://codereview.chromium.org/2832018/show -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev

[v8-dev] Made scanner follow coding style. (issue2832018)

2010-06-22 Thread lrn
Reviewers: Rico, Description: Made scanner follow coding style. Please review this at http://codereview.chromium.org/2832018/show Affected files: M src/scanner.h M src/scanner.cc -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev

[v8-dev] [v8] r4915 committed - Augment trace-ic to provide information on patching inlined loads....

2010-06-22 Thread codesite-noreply
Revision: 4915 Author: erik.co...@gmail.com Date: Tue Jun 22 05:12:32 2010 Log: Augment trace-ic to provide information on patching inlined loads. Review URL: http://codereview.chromium.org/2818023 http://code.google.com/p/v8/source/detail?r=4915 Modified: /branches/bleeding_edge/src/ic.cc

[v8-dev] Re: Augment trace-ic to provide information on patching inlined loads. (issue2818023)

2010-06-22 Thread erik . corry
I didn't do the other inlined cases. Another time perhaps. I put the logging in ifdefs so it doesn't go in the release version. http://codereview.chromium.org/2818023/diff/1/2 File src/ic.cc (right): http://codereview.chromium.org/2818023/diff/1/2#newcode734 src/ic.cc:734: if (FLAG_trace_ic)

[v8-dev] Re: Remove redundant checks in and around GenerateDictionaryLoad.... (issue2801007)

2010-06-22 Thread kaznacheev
http://codereview.chromium.org/2801007/show -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev

[v8-dev] Re: Augment trace-ic to provide information on patching inlined loads. (issue2818023)

2010-06-22 Thread sgjesse
LGTM How about the other types which can be inlined? http://codereview.chromium.org/2818023/diff/1/2 File src/ic.cc (right): http://codereview.chromium.org/2818023/diff/1/2#newcode734 src/ic.cc:734: if (FLAG_trace_ic) PrintF("[LoadIC : patch %s ]\n", *name->ToCString()); Maybe remove this line

[v8-dev] Re: Make the apply.js unit test more resilient to differing stack positions.... (issue2835014)

2010-06-22 Thread erik . corry
LGTM http://codereview.chromium.org/2835014/show -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev

[v8-dev] Augment trace-ic to provide information on patching inlined loads. (issue2818023)

2010-06-22 Thread erik . corry
Reviewers: Søren Gjesse, Description: Augment trace-ic to provide information on patching inlined loads. Please review this at http://codereview.chromium.org/2818023/show SVN Base: http://v8.googlecode.com/svn/branches/bleeding_edge/ Affected files: M src/ic.cc Index: src/ic.cc ===

[v8-dev] [v8] r4914 committed - X64: Change strategy for spilling to match ia32. It's just better....

2010-06-22 Thread codesite-noreply
Revision: 4914 Author: l...@chromium.org Date: Tue Jun 22 03:07:57 2010 Log: X64: Change strategy for spilling to match ia32. It's just better. Align deferred code blocks to 16-byte address boundaries. Review URL: http://codereview.chromium.org/2855018 http://code.google.com/p/v8/source/detail?r=

[v8-dev] Re: X64: Change strategy for spilling to match ia32. It's just better. (issue2855018)

2010-06-22 Thread whesse
LGTM. http://codereview.chromium.org/2855018/diff/1/10 File src/x64/virtual-frame-x64.cc (right): http://codereview.chromium.org/2855018/diff/1/10#newcode964 src/x64/virtual-frame-x64.cc:964: // TODO(lrn): Can we assert (end > stack_pointer_)? Resolve this TODO now. I doubt it, and it probably

[v8-dev] X64: Change strategy for spilling to match ia32. It's just better. (issue2855018)

2010-06-22 Thread lrn
Reviewers: William Hesse, Description: X64: Change strategy for spilling to match ia32. It's just better. Align deferred code blocks to 16-byte address boundaries. Please review this at http://codereview.chromium.org/2855018/show Affected files: M src/arm/assembler-arm.h M src/arm/assembler

[v8-dev] [v8] r4913 committed - Add movw and movt support for ARMv7. This includes some code from...

2010-06-22 Thread codesite-noreply
Revision: 4913 Author: erik.co...@gmail.com Date: Tue Jun 22 01:38:32 2010 Log: Add movw and movt support for ARMv7. This includes some code from Zhang Kun. For now we only emit movw and movt in places where no relocation is needed. Small performance boost (around 0.5%). Also adds support for t

[v8-dev] Re: Issue 746 in v8: Inline fast-pass MUL binary operation on ARM

2010-06-22 Thread codesite-noreply
Comment #1 on issue 746 by a...@chromium.org: Inline fast-pass MUL binary operation on ARM http://code.google.com/p/v8/issues/detail?id=746 This sounds interesting. Could you upload your change for comments in the code review tool? It will be much easier discussing your proposal through c