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

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

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

2010-06-23 Thread sgjesse
LGTM http://codereview.chromium.org/2811021/show -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev

[v8-dev] Re: Fixing the regression introduced in r4716.... (issue2862028)

2010-06-23 Thread sgjesse
LGTM, well spotted. http://codereview.chromium.org/2862028/show -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev

[v8-dev] Re: Issue 49 in v8: v8 does not use correct Integer types.

2010-06-23 Thread codesite-noreply
Comment #2 on issue 49 by preston.bannister: v8 does not use correct Integer types. http://code.google.com/p/v8/issues/detail?id=49 I disagree with counting this as a bug. Over the years I have written a lot of portable code - for 16-bit, 32-bit, and 64-bit machines, different compilers, a

[v8-dev] Re: [Isolates] Move Logger to isolate, keep Log a per-process instance. (issue2813030)

2010-06-23 Thread dimich
Adding Mikhail in case he wanted to take a look. http://codereview.chromium.org/2813030/show -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev

[v8-dev] Re: [Isolates] Move Logger to isolate, keep Log a per-process instance. (issue2813030)

2010-06-23 Thread dimich
Oops, wrong Vitaly. Now with the right one. http://codereview.chromium.org/2813030/show -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev

[v8-dev] Inline fast-pass MUL binary operation (issue2868018)

2010-06-23 Thread zhangk
Reviewers: mads.ager, Message: We inline the fast-pass MUL binaryop when it is called in a loop to improve the performanc while controling the code size. The MUL stub could be called by different functions and we only inline MUL for some cases, so we must keep the original MUL stub untouche

[v8-dev] Move Logger to isolate, keep Log a per-process instance. (issue2813030)

2010-06-23 Thread dimich
Reviewers: vitaflo, Description: Move Logger to isolate, keep Log a per-process instance. Logger and Profiler are now per-isolate. Log (the actual memory buffer or file) is per-process, already threadsafe. Accessed by multiple Loggers. Since there is a callback from a single Log to multiple Logge

[v8-dev] Issue 751 in v8: JSON.parse must reject unencoded tab characters

2010-06-23 Thread codesite-noreply
Status: New Owner: New issue 751 by erights: JSON.parse must reject unencoded tab characters http://code.google.com/p/v8/issues/detail?id=751 See https://mail.mozilla.org/pipermail/es-discuss/2010-June/011412.html and the surrounding thread context. -- v8-dev mailing list v8-dev@google

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

2010-06-23 Thread lukezarko
http://codereview.chromium.org/2841023/diff/21002/34016 File src/heap.cc (right): http://codereview.chromium.org/2841023/diff/21002/34016#newcode4484 src/heap.cc:4484: void Heap::PreInit() { On 2010/06/23 13:55:08, Vitaly wrote: Are there any other planned usages of PreInit? If not, make debug

[v8-dev] Issue 748 in v8: Chrome crasher related to inspector timeline on Linux 64 bit

2010-06-23 Thread codesite-noreply
Status: New Owner: New issue 748 by con...@google.com: Chrome crasher related to inspector timeline on Linux 64 bit http://code.google.com/p/v8/issues/detail?id=748 Detailed description of the issue. Chrome Dev Channel crashes when trying to run the inspector timeline on some pages. F

[v8-dev] [v8] r4931 committed - [Isolates] Make statics from BootstrapperActive to be instance members...

2010-06-23 Thread codesite-noreply
Revision: 4931 Author: vita...@chromium.org Date: Wed Jun 23 09:21:33 2010 Log: [Isolates] Make statics from BootstrapperActive to be instance members of Bootstrapper. Bootstrapper::IsActive is also instance method now. Landing patch by Maxim.Mossienko with minor style changes. Original review:

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

2010-06-23 Thread vitalyr
LGTM. Landed with the proposed style fixes in http://code.google.com/p/v8/source/detail?r=4931 Thanks, Vitaly http://codereview.chromium.org/2813026/diff/1/9 File src/bootstrapper.h (right): http://codereview.chromium.org/2813026/diff/1/9#newcode35 src/bootstrapper.h:35: class BootstrapperAct

[v8-dev] [v8] r4930 committed - X64: Change some fpu operations to use XMM registers....

2010-06-23 Thread codesite-noreply
Revision: 4930 Author: l...@chromium.org Date: Wed Jun 23 07:05:18 2010 Log: X64: Change some fpu operations to use XMM registers. Review URL: http://codereview.chromium.org/2827022 http://code.google.com/p/v8/source/detail?r=4930 Modified: /branches/bleeding_edge/src/x64/assembler-x64.cc /bra

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

2010-06-23 Thread mnaganov
http://codereview.chromium.org/2841023/diff/21002/34021 File src/log.cc (right): http://codereview.chromium.org/2841023/diff/21002/34021#newcode311 src/log.cc:311: if (Logger::profiler_) { On 2010/06/23 13:57:45, Michail Naganov wrote: This seems strange. Logger::profiler_ is initialized with a

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

2010-06-23 Thread mnaganov
http://codereview.chromium.org/2841023/diff/21002/34021 File src/log.cc (right): http://codereview.chromium.org/2841023/diff/21002/34021#newcode311 src/log.cc:311: if (Logger::profiler_) { This seems strange. Logger::profiler_ is initialized with a Profiler instance. That means, instead of Logge

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

2010-06-23 Thread vitalyr
I couldn't get rid of a feeling that this is really a few independent changes put together in one patch. I'm not against huge patches that make sense, i.e. make logical steps forward, but huge patches like this one risk decreasing the quality of reviews. Once again let me remind you of git sv

[v8-dev] [v8] r4929 committed - ARM: Fix bug introduced in 4783 (2.2.15) that caused the...

2010-06-23 Thread codesite-noreply
Revision: 4929 Author: erik.co...@gmail.com Date: Wed Jun 23 06:44:11 2010 Log: ARM: Fix bug introduced in 4783 (2.2.15) that caused the result of 1 << x to be miscalculated for some inputs. Review URL: http://codereview.chromium.org/2848021 http://code.google.com/p/v8/source/detail?r=4929 Modifi

[v8-dev] Re: X64: Change some fpu operations to use XMM registers. (issue2827022)

2010-06-23 Thread whesse
LGTM. http://codereview.chromium.org/2827022/diff/1/6 File src/x64/ic-x64.cc (right): http://codereview.chromium.org/2827022/diff/1/6#newcode1187 src/x64/ic-x64.cc:1187: __ xorl(rdx, rdx); Why not Set(rdx, 0)? That automatically does the correct thing. http://codereview.chromium.org/2827022/s

[v8-dev] Re: ARM: Fix bug introduced in 4783 (2.2.15) that caused the... (issue2848021)

2010-06-23 Thread whesse
LGTM. http://codereview.chromium.org/2848021/show -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev

[v8-dev] Port optimization of comparison with a trivial LHS from ia32 to x64. (issue2868028)

2010-06-23 Thread whesse
Reviewers: Erik Corry, Description: Port optimization of comparison with a trivial LHS from ia32 to x64. Please review this at http://codereview.chromium.org/2868028/show SVN Base: http://v8.googlecode.com/svn/branches/bleeding_edge/ Affected files: M src/x64/codegen-x64.cc Index: src/

[v8-dev] ARM: Fix bug introduced in 4783 (2.2.15) that caused the... (issue2848021)

2010-06-23 Thread erik . corry
Reviewers: William Hesse, Description: ARM: Fix bug introduced in 4783 (2.2.15) that caused the result of 1 << x to be miscalculated for some inputs. Please review this at http://codereview.chromium.org/2848021/show SVN Base: http://v8.googlecode.com/svn/branches/bleeding_edge/ Affected files:

[v8-dev] X64: Change some fpu operations to use XMM registers. (issue2827022)

2010-06-23 Thread lrn
Reviewers: William Hesse, Description: X64: Change some fpu operations to use XMM registers. Please review this at http://codereview.chromium.org/2827022/show Affected files: M src/x64/assembler-x64.h M src/x64/assembler-x64.cc M src/x64/codegen-x64.cc M src/x64/disasm-x64.cc M src/x6

[v8-dev] Take survival rates of young objects into account when choosing old generation limits. (issue2809032)

2010-06-23 Thread vegorov
Reviewers: , Description: Take survival rates of young objects into account when choosing old generation limits. Stable high survival rates of young objects both during partial and full collection indicate that mutator is either building or modifying a structure with a long lifetime. In this

[v8-dev] [v8] r4928 committed - X64: A bunch of small fixes....

2010-06-23 Thread codesite-noreply
Revision: 4928 Author: l...@chromium.org Date: Wed Jun 23 04:48:30 2010 Log: X64: A bunch of small fixes. Make push/pop use emit_optional_rex32. Fix bug in disassembler (swapped name of comisd/ucomisd). Use fstp in FCmp macro. Review URL: http://codereview.chromium.org/2818026 http://code.google.

[v8-dev] Re: X64: A bunch of small fixes. (issue2818026)

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

[v8-dev] Re: X64: A bunch of small fixes. (issue2818026)

2010-06-23 Thread lrn
http://codereview.chromium.org/2818026/diff/1/2 File src/x64/assembler-x64.cc (right): http://codereview.chromium.org/2818026/diff/1/2#newcode381 src/x64/assembler-x64.cc:381: nop(9); Whoops, fixed. http://codereview.chromium.org/2818026/diff/1/3 File src/x64/assembler-x64.h (right): http://co

[v8-dev] Re: X64: A bunch of small fixes. (issue2818026)

2010-06-23 Thread ricow
LGTM with loop fixed. http://codereview.chromium.org/2818026/diff/1/2 File src/x64/assembler-x64.cc (right): http://codereview.chromium.org/2818026/diff/1/2#newcode381 src/x64/assembler-x64.cc:381: nop(9); if delta is >= 9 this will never terminate http://codereview.chromium.org/2818026/diff/1

[v8-dev] X64: A bunch of small fixes. (issue2818026)

2010-06-23 Thread lrn
Reviewers: Rico, Description: X64: A bunch of small fixes. Make push/pop use emit_optional_rex32. Fix bug in disassembler (swapped name of comisd/ucomisd). Use fstp in FCmp macro. Please review this at http://codereview.chromium.org/2818026/show Affected files: M src/x64/assembler-x64.h M s

[v8-dev] [v8] r4927 committed - Use SSE2 registers when comparing identical heap numbers on X64....

2010-06-23 Thread codesite-noreply
Revision: 4927 Author: whe...@chromium.org Date: Wed Jun 23 02:21:32 2010 Log: Use SSE2 registers when comparing identical heap numbers on X64. Review URL: http://codereview.chromium.org/2850018 http://code.google.com/p/v8/source/detail?r=4927 Modified: /branches/bleeding_edge/src/x64/codegen-x6

[v8-dev] [v8] r4925 committed - Tagging version 2.2.19

2010-06-23 Thread codesite-noreply
Revision: 4925 Author: ri...@chromium.org Date: Wed Jun 23 02:08:34 2010 Log: Tagging version 2.2.19 http://code.google.com/p/v8/source/detail?r=4925 Added: /tags/2.2.19 -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev

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

2010-06-23 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: Remove redundant checks in and around GenerateDictionaryLoad.... (issue2801007)

2010-06-23 Thread kaznacheev
http://codereview.chromium.org/2801007/diff/1/2 File src/ia32/ic-ia32.cc (right): http://codereview.chromium.org/2801007/diff/1/2#newcode98 src/ia32/ic-ia32.cc:98: // Appropriate access checks must be done before entering this code. On 2010/06/17 11:37:04, Mads Ager wrote: You need to spell out

[v8-dev] [v8] r4923 committed - Prepare push to trunk. Now working on version 2.2.20....

2010-06-23 Thread codesite-noreply
Revision: 4923 Author: ri...@chromium.org Date: Wed Jun 23 01:51:53 2010 Log: Prepare push to trunk. Now working on version 2.2.20. Review URL: http://codereview.chromium.org/2815025 http://code.google.com/p/v8/source/detail?r=4923 Modified: /branches/bleeding_edge/ChangeLog /branches/bleeding_

[v8-dev] Fixing the regression introduced in r4716.... (issue2862028)

2010-06-23 Thread kaznacheev
Reviewers: Søren Gjesse, Description: Fixing the regression introduced in r4716. The regression made 2 tests fail on ia32 with --always-full-compiler. Please review this at http://codereview.chromium.org/2862028/show SVN Base: http://v8.googlecode.com/svn/branches/bleeding_edge/ Affected fil

[v8-dev] Re: Prepare push to trunk. Now working on version 2.2.20. (issue2815025)

2010-06-23 Thread erik . corry
4898 sounds as if it is something that embedders need to know. Otherwise LGTM http://codereview.chromium.org/2815025/show -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev

Re: [v8-dev] Prepare push to trunk. Now working on version 2.2.20. (issue2815025)

2010-06-23 Thread Kasper Lund
On Wed, Jun 23, 2010 at 10:20 AM, wrote: > +        Added expose-externalize-string flag for testing extinsions. extinsions => extensions -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev

[v8-dev] Prepare push to trunk. Now working on version 2.2.20. (issue2815025)

2010-06-23 Thread ricow
Reviewers: Erik Corry, Description: Prepare push to trunk. Now working on version 2.2.20. Please review this at http://codereview.chromium.org/2815025/show SVN Base: http://v8.googlecode.com/svn/branches/bleeding_edge/ Affected files: M ChangeLog M src/version.cc Index: ChangeLog

[v8-dev] [v8] r4922 committed - Add regression test for the code flushing in issue 474 (which was...

2010-06-23 Thread codesite-noreply
Revision: 4922 Author: ri...@chromium.org Date: Wed Jun 23 01:02:06 2010 Log: Add regression test for the code flushing in issue 474 (which was fixed in revision 4921). This also enables codeflushing by default. Review URL: http://codereview.chromium.org/2829020 http://code.google.com/p/v8/sourc

[v8-dev] Re: Add regression test for the code flushing in issue 474 (which was... (issue2829020)

2010-06-23 Thread kasperl
LGTM. http://codereview.chromium.org/2829020/diff/1/3 File test/mjsunit/regress/regress-747.js (right): http://codereview.chromium.org/2829020/diff/1/3#newcode39 test/mjsunit/regress/regress-747.js:39: try{ Space after try. http://codereview.chromium.org/2829020/diff/1/3#newcode52 test/mjsunit

[v8-dev] Add regression test for the code flushing in issue 474 (which was... (issue2829020)

2010-06-23 Thread ricow
Reviewers: Kasper Lund, Message: Small addition to last commit, including a regression test. Description: Add regression test for the code flushing in issue 474 (which was fixed in revision 4921). This also enables codeflushing by default. Please review this at http://codereview.chromium.org/

[v8-dev] Re: Use SSE2 registers when comparing identical heap numbers on X64. (issue2850018)

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

[v8-dev] Issue 747 in v8: Code flushing during gc will flush code that has heap allocated locals

2010-06-23 Thread codesite-noreply
Status: Accepted Owner: ri...@chromium.org Labels: Type-Bug Priority-Medium New issue 747 by ri...@chromium.org: Code flushing during gc will flush code that has heap allocated locals http://code.google.com/p/v8/issues/detail?id=747 When doing code flushing there is no check for heap allocate

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

2010-06-23 Thread ager
LGTM! http://codereview.chromium.org/2801007/diff/5001/6002 File src/arm/ic-arm.cc (right): http://codereview.chromium.org/2801007/diff/5001/6002#newcode53 src/arm/ic-arm.cc:53: Label* miss) { miss -> global_object? http://codereview.chromium.org/2801007/diff/5001/6002#newcode65 src/arm/ic-arm

[v8-dev] [v8] r4921 committed - Only flush code when there are no heap allocated locals in the scopein...

2010-06-23 Thread codesite-noreply
Revision: 4921 Author: ri...@chromium.org Date: Wed Jun 23 00:16:09 2010 Log: Only flush code when there are no heap allocated locals in the scopeinfo. When flushing code we can potentially flush code with a scopeinfo that we later need to resolve variables. This makes an explicit check that t

[v8-dev] Re: Only flush code when there are no heap allocated locals in the scopeinfo.... (issue2836021)

2010-06-23 Thread ricow
http://codereview.chromium.org/2836021/diff/5001/6001 File src/heap.cc (right): http://codereview.chromium.org/2836021/diff/5001/6001#newcode2270 src/heap.cc:2270: if ( ScopeInfo<>::HasHeapAllocatedLocals(function_info->code())) On 2010/06/23 07:14:27, Kasper Lund wrote: Remove space before Sco

[v8-dev] Re: Only flush code when there are no heap allocated locals in the scopeinfo.... (issue2836021)

2010-06-23 Thread kasperl
LGTM. http://codereview.chromium.org/2836021/diff/5001/6001 File src/heap.cc (right): http://codereview.chromium.org/2836021/diff/5001/6001#newcode2270 src/heap.cc:2270: if ( ScopeInfo<>::HasHeapAllocatedLocals(function_info->code())) Remove space before ScopeInfo. http://codereview.chromium.o

[v8-dev] [v8] r4920 committed - Lower waste from alignment of deferred code blocks....

2010-06-23 Thread codesite-noreply
Revision: 4920 Author: l...@chromium.org Date: Wed Jun 23 00:03:34 2010 Log: Lower waste from alignment of deferred code blocks. Some ARM chips load instructions 8 byte at a time. Review URL: http://codereview.chromium.org/2809029 http://code.google.com/p/v8/source/detail?r=4920 Modified: /bran

[v8-dev] Only flush code when there are no heap allocated locals in the scopeinfo.... (issue2836021)

2010-06-23 Thread ricow
Reviewers: Kasper Lund, Description: Only flush code when there are no heap allocated locals in the scopeinfo. When flushing code we can potentially flush code with a scopeinfo that we later need to resolve variables. This makes an explicit check that there are heap allocated locals in the scope