[v8-dev] Re: Fix BinaryOpIC implementation on ARM.... (issue1687005)

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

[v8-dev] [v8] r4467 committed - Support multi-chunk differences...

2010-04-21 Thread codesite-noreply
Revision: 4467 Author: peter.ry...@gmail.com Date: Wed Apr 21 09:59:58 2010 Log: Support multi-chunk differences Review URL: http://codereview.chromium.org/1672006 http://code.google.com/p/v8/source/detail?r=4467 Modified: /branches/bleeding_edge/src/liveedit-debugger.js /branches/bleeding_edg

[v8-dev] Re: Support multi-chunk differences (issue1672006)

2010-04-21 Thread peter . rybin
http://codereview.chromium.org/1672006/diff/9001/10001 File src/liveedit-debugger.js (right): http://codereview.chromium.org/1672006/diff/9001/10001#newcode50 src/liveedit-debugger.js:50: // This API is a legacy and is obsolete. On 2010/04/21 13:41:53, Søren Gjesse wrote: Why not remove it then

[v8-dev] Re: Add inlining of property load on ARM... (issue1715003)

2010-04-21 Thread ager
LGTM http://codereview.chromium.org/1715003/diff/9001/10006 File src/arm/assembler-arm.cc (right): http://codereview.chromium.org/1715003/diff/9001/10006#newcode359 src/arm/assembler-arm.cc:359: return ((instr & Imm24Mask) << 8) >> 6; Maybe you should add a comment with the explanation that you

[v8-dev] Re: Issue 683 in v8: Current order of initialization in CpuProfiler::StartProcessorIfNotStarted can lead to VM deadlock.

2010-04-21 Thread codesite-noreply
Updates: Status: Fixed Comment #1 on issue 683 by mikhail.naganov: Current order of initialization in CpuProfiler::StartProcessorIfNotStarted can lead to VM deadlock. http://code.google.com/p/v8/issues/detail?id=683 Fixed in http://code.google.com/p/v8/source/detail?r=4464 -- You rec

[v8-dev] [v8] r4466 committed - Tag r4465 as version 2.2.4.2

2010-04-21 Thread codesite-noreply
Revision: 4466 Author: mikhail.naga...@gmail.com Date: Wed Apr 21 07:24:14 2010 Log: Tag r4465 as version 2.2.4.2 http://code.google.com/p/v8/source/detail?r=4466 Added: /tags/2.2.4.2 -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev

[v8-dev] [v8] r4465 committed - Patch trunk to version 2.2.4.2, pulling in change 4464....

2010-04-21 Thread codesite-noreply
Revision: 4465 Author: mikhail.naga...@gmail.com Date: Wed Apr 21 07:22:54 2010 Log: Patch trunk to version 2.2.4.2, pulling in change 4464. Review URL: http://codereview.chromium.org/1730003 http://code.google.com/p/v8/source/detail?r=4465 Modified: /trunk/src/cpu-profiler.cc /trunk/src/versi

[v8-dev] Re: Patch trunk to version 2.2.4.2, pulling in change 4464. (issue1730003)

2010-04-21 Thread ager
LGTM http://codereview.chromium.org/1730003/show -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev

[v8-dev] Patch trunk to version 2.2.4.2, pulling in change 4464. (issue1730003)

2010-04-21 Thread mnaganov
Reviewers: Mads Ager, Description: Patch trunk to version 2.2.4.2, pulling in change 4464. Please review this at http://codereview.chromium.org/1730003/show SVN Base: http://v8.googlecode.com/svn/trunk/ Affected files: M src/cpu-profiler.cc M src/version.cc Index: src/cpu-profil

[v8-dev] [v8] r4464 committed - Fix issue 683: change the order of CPU profiler setup actions....

2010-04-21 Thread codesite-noreply
Revision: 4464 Author: mikhail.naga...@gmail.com Date: Wed Apr 21 07:07:13 2010 Log: Fix issue 683: change the order of CPU profiler setup actions. BUG=683 Review URL: http://codereview.chromium.org/1756003 http://code.google.com/p/v8/source/detail?r=4464 Modified: /branches/bleeding_edge/src/

[v8-dev] Re: Added ability to remove prototype from function. In this case, [[Construct]] ... (issue1722003)

2010-04-21 Thread dgozman
http://codereview.chromium.org/1722003/diff/1/4 File src/objects.cc (right): http://codereview.chromium.org/1722003/diff/1/4#newcode4935 src/objects.cc:4935: Object* JSFunction::RemovePrototype() { On 2010/04/21 13:51:12, antonm wrote: is it fine that we create many maps---one per removed proto

[v8-dev] Re: Fix issue 683: change the order of CPU profiler setup actions. (issue1756003)

2010-04-21 Thread ager
LGTM http://codereview.chromium.org/1756003/diff/1/2 File src/cpu-profiler.cc (right): http://codereview.chromium.org/1756003/diff/1/2#newcode424 src/cpu-profiler.cc:424: // It is important to have it started prior to logging, see issue 683. Could you put in the URL for the issue to make it com

[v8-dev] Fix issue 683: change the order of CPU profiler setup actions. (issue1756003)

2010-04-21 Thread mnaganov
Reviewers: Mads Ager, Description: Fix issue 683: change the order of CPU profiler setup actions. BUG=683 Please review this at http://codereview.chromium.org/1756003/show Affected files: M src/cpu-profiler.cc Index: src/cpu-profiler.cc diff --git a/src/cpu-profiler.cc b/src/cpu-profiler.c

[v8-dev] Issue 683 in v8: Current order of initialization in CpuProfiler::StartProcessorIfNotStarted can lead to VM deadlock.

2010-04-21 Thread codesite-noreply
Status: Accepted Owner: mikhail.naganov Labels: Type-Bug Priority-High New issue 683 by mikhail.naganov: Current order of initialization in CpuProfiler::StartProcessorIfNotStarted can lead to VM deadlock. http://code.google.com/p/v8/issues/detail?id=683 Currently, CpuProfiler::StartProcessorI

[v8-dev] Re: Added ability to remove prototype from function. In this case, [[Construct]] ... (issue1722003)

2010-04-21 Thread antonm
Drive by question http://codereview.chromium.org/1722003/diff/1/4 File src/objects.cc (right): http://codereview.chromium.org/1722003/diff/1/4#newcode4935 src/objects.cc:4935: Object* JSFunction::RemovePrototype() { is it fine that we create many maps---one per removed prototype? http://codere

[v8-dev] Re: Support multi-chunk differences (issue1672006)

2010-04-21 Thread sgjesse
LGTM http://codereview.chromium.org/1672006/diff/9001/10001 File src/liveedit-debugger.js (right): http://codereview.chromium.org/1672006/diff/9001/10001#newcode50 src/liveedit-debugger.js:50: // This API is a legacy and is obsolete. Why not remove it then? http://codereview.chromium.org/16720

[v8-dev] [v8] r4463 committed - Port inlined quick equality check for non-NaN to x64....

2010-04-21 Thread codesite-noreply
Revision: 4463 Author: whe...@chromium.org Date: Wed Apr 21 06:33:36 2010 Log: Port inlined quick equality check for non-NaN to x64. Review URL: http://codereview.chromium.org/1756002 http://code.google.com/p/v8/source/detail?r=4463 Modified: /branches/bleeding_edge/src/ia32/codegen-ia32.cc /br

[v8-dev] Re: Issue 682 in v8: Date failures on arm

2010-04-21 Thread codesite-noreply
Comment #2 on issue 682 by ri...@chromium.org: Date failures on arm http://code.google.com/p/v8/issues/detail?id=682 This bug is related to the machines running the tests has either UTC or Chinese timezone (which, like UTC has no daylight saving time). This bug is not in v8, but rather in t

[v8-dev] Re: Issue 682 in v8: Date failures on arm

2010-04-21 Thread codesite-noreply
Updates: Status: WorkingAsIntended Comment #1 on issue 682 by ri...@chromium.org: Date failures on arm http://code.google.com/p/v8/issues/detail?id=682 This bug is related to the machines running on tests has either UTC or Chinese timezone (which, like UTC has no daylight saving time).

[v8-dev] Re: Port inlined quick equality check for non-NaN to x64. (issue1756002)

2010-04-21 Thread ager
LGTM http://codereview.chromium.org/1756002/show -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev

[v8-dev] Port inlined quick equality check for non-NaN to x64. (issue1756002)

2010-04-21 Thread whesse
Reviewers: Mads Ager, Message: Simple cl for you. Description: Port inlined quick equality check for non-NaN to x64. Please review this at http://codereview.chromium.org/1756002/show SVN Base: http://v8.googlecode.com/svn/branches/bleeding_edge/ Affected files: M src/ia32/codegen-ia32.c

[v8-dev] Add inlining of property load on ARM... (issue1715003)

2010-04-21 Thread sgjesse
Reviewers: Erik Corry, Mads Ager, Description: Add inlining of property load on ARM Generate inlined named property load for in-object properties. This uses the same mechanism as on the Intel platforms with the map check and load instruction of the inlined code being patched by the inli

[v8-dev] Added ability to remove prototype from function. In this case, [[Construct]] ... (issue1722003)

2010-04-21 Thread dgozman
Reviewers: Mads Ager, Description: Added ability to remove prototype from function. In this case, [[Construct]] from function will not be allowed. Added runtime function %FunctionRemovePrototype for this. Removed prototypes from all builtin functions. Some sputnik tests marked as fixed. Please

[v8-dev] Fix BinaryOpIC implementation on ARM.... (issue1687005)

2010-04-21 Thread kaznacheev
Reviewers: Erik Corry, Description: Fix BinaryOpIC implementation on ARM. On a pair of smis HEAP_NUMBERS stub is significantly slower than GENERIC. This slows down some tests dramatically (crypto-aes from SunSpider). With this change HEAP_NUMBERS stub switches to GENERIC stub the first time

[v8-dev] [v8] r4462 committed - - Fix unitialized variable error found by compiler warning....

2010-04-21 Thread codesite-noreply
Revision: 4462 Author: ipo...@chromium.org Date: Wed Apr 21 05:16:36 2010 Log: - Fix unitialized variable error found by compiler warning. Review URL: http://codereview.chromium.org/174 http://code.google.com/p/v8/source/detail?r=4462 Modified: /branches/bleeding_edge/src/spaces.cc ===

[v8-dev] Re: - Fix unitialized variable error found by compiler warning. (issue1700004)

2010-04-21 Thread Anton Muhin
lgtm On Wed, Apr 21, 2010 at 4:13 PM, wrote: > Reviewers: antonm, > > Description: > - Fix unitialized variable error found by compiler warning. > > > Please review this at http://codereview.chromium.org/174/show > > SVN Base: http://v8.googlecode.com/svn/branches/bleeding_edge/ > > Affected

[v8-dev] - Fix unitialized variable error found by compiler warning. (issue1700004)

2010-04-21 Thread iposva
Reviewers: antonm, Description: - Fix unitialized variable error found by compiler warning. Please review this at http://codereview.chromium.org/174/show SVN Base: http://v8.googlecode.com/svn/branches/bleeding_edge/ Affected files: M src/spaces.cc Index: src/spaces.cc ===

[v8-dev] Re: Bring r4460 to trunk. (issue1689004)

2010-04-21 Thread Anton Muhin
Thanks a lot, Lasse. yours, anton. On Wed, Apr 21, 2010 at 3:56 PM, wrote: > LGTM > > http://codereview.chromium.org/1689004/show > -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev

[v8-dev] [v8] r4461 committed - Bring r4460 to trunk....

2010-04-21 Thread codesite-noreply
Revision: 4461 Author: ant...@chromium.org Date: Wed Apr 21 05:00:05 2010 Log: Bring r4460 to trunk. This fixes an overwrite past the end of cache. Review URL: http://codereview.chromium.org/1689004 http://code.google.com/p/v8/source/detail?r=4461 Modified: /trunk/src/objects.h /trunk/src/run

[v8-dev] Re: Bring r4460 to trunk. (issue1689004)

2010-04-21 Thread lrn
LGTM http://codereview.chromium.org/1689004/show -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev

[v8-dev] Bring r4460 to trunk. (issue1689004)

2010-04-21 Thread antonm
Reviewers: Lasse Reichstein, Mads Ager, Message: Lasse, Mads, may you have a look? Description: Bring r4460 to trunk. This fixes an overwrite past the end of cache. Please review this at http://codereview.chromium.org/1689004/show SVN Base: http://v8.googlecode.com/svn/trunk/ Affected file

[v8-dev] [v8] r4460 committed - Fix one off error....

2010-04-21 Thread codesite-noreply
Revision: 4460 Author: ant...@chromium.org Date: Wed Apr 21 04:13:53 2010 Log: Fix one off error. Proper condition to start eviction is when next possible index is equal to cache length. Review URL: http://codereview.chromium.org/1709001 http://code.google.com/p/v8/source/detail?r=4460 Modified

[v8-dev] Re: Don't share function result caches between contexts. (issue1731002)

2010-04-21 Thread antonm
Thanks a lot for spotting this and sorry for introducing the trouble. Almost LGTM. Am I missing something or src/bootstrapper.cc should be update as well to build caches per context? http://codereview.chromium.org/1731002/diff/1/2 File src/arm/codegen-arm.cc (right): http://codereview.chro

[v8-dev] Re: Fix one off error. (issue1709001)

2010-04-21 Thread antonm
Lasse, thanks lot for review! I'm submitting it with one change in 5 mins (to run the tests) as I want it on trunk soon. And you are quite right: my mistake made the code overwrite values past the fixed array, so poor maps :( http://codereview.chromium.org/1709001/diff/1/2 File src/runtime.

[v8-dev] [v8] r4459 committed - Port bugfix in revision 4449 to 2.1 branch....

2010-04-21 Thread codesite-noreply
Revision: 4459 Author: l...@chromium.org Date: Wed Apr 21 03:24:56 2010 Log: Port bugfix in revision 4449 to 2.1 branch. Review URL: http://codereview.chromium.org/1725002 http://code.google.com/p/v8/source/detail?r=4459 Modified: /branches/2.1/src/runtime.cc /branches/2.1/src/string.js /bran

[v8-dev] Re: Port bugfix in revision 4449 to 2.1 branch. (issue1725002)

2010-04-21 Thread ager
LGTM http://codereview.chromium.org/1725002/show -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev

[v8-dev] Port bugfix in revision 4449 to 2.1 branch. (issue1725002)

2010-04-21 Thread lrn
Reviewers: Mads Ager, Description: Port bugfix in revision 4449 to 2.1 branch. Please review this at http://codereview.chromium.org/1725002/show Affected files: M src/runtime.cc M src/string.js M src/version.cc -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/grou

[v8-dev] [v8] r4458 committed - Fix lint errors...

2010-04-21 Thread codesite-noreply
Revision: 4458 Author: sgje...@chromium.org Date: Wed Apr 21 03:20:55 2010 Log: Fix lint errors tbr=erik.co...@gmail.com Review URL: http://codereview.chromium.org/1749002 http://code.google.com/p/v8/source/detail?r=4458 Modified: /branches/bleeding_edge/src/arm/assembler-arm.h /branches/bleed

[v8-dev] Fix lint errors (issue1749002)

2010-04-21 Thread sgjesse
Reviewers: Erik Corry, Description: Fix lint errors tbr=erik.co...@gmail.com Please review this at http://codereview.chromium.org/1749002/show SVN Base: http://v8.googlecode.com/svn/branches/bleeding_edge/ Affected files: M src/arm/assembler-arm.h M src/arm/codegen-arm.cc M

[v8-dev] [v8] r4457 committed - Update comment to provide the correct usage of the Disassembler class....

2010-04-21 Thread codesite-noreply
Revision: 4457 Author: sgje...@chromium.org Date: Wed Apr 21 02:45:06 2010 Log: Update comment to provide the correct usage of the Disassembler class. tbr=erik.co...@gmail.com Review URL: http://codereview.chromium.org/1755001 http://code.google.com/p/v8/source/detail?r=4457 Modified: /branches

[v8-dev] [v8] r4456 committed - Use an object to control the blocking of the constant pool...

2010-04-21 Thread codesite-noreply
Revision: 4456 Author: sgje...@chromium.org Date: Wed Apr 21 02:43:45 2010 Log: Use an object to control the blocking of the constant pool Instead of indicating for how many instructions the constant pool needs to be blocked the constant pool is now blocked while at least one instance of Scop

[v8-dev] Re: Use an object to control the blocking of the constant pool... (issue1673006)

2010-04-21 Thread sgjesse
http://codereview.chromium.org/1673006/diff/15001/7002 File src/arm/assembler-arm.h (right): http://codereview.chromium.org/1673006/diff/15001/7002#newcode929 src/arm/assembler-arm.h:929: class ScopedConstPoolBlocker { On 2010/04/21 09:28:50, Mads Ager wrote: I think we usually call these class

[v8-dev] Update comment to provide the correct usage of the Disassembler class.... (issue1755001)

2010-04-21 Thread sgjesse
Reviewers: Erik Corry, Description: Update comment to provide the correct usage of the Disassembler class. tbr=erik.co...@gmail.com Please review this at http://codereview.chromium.org/1755001/show SVN Base: http://v8.googlecode.com/svn/branches/bleeding_edge/ Affected files: M src/arm/

[v8-dev] Re: Use an object to control the blocking of the constant pool... (issue1673006)

2010-04-21 Thread ager
STV! http://codereview.chromium.org/1673006/diff/15001/7002 File src/arm/assembler-arm.h (right): http://codereview.chromium.org/1673006/diff/15001/7002#newcode929 src/arm/assembler-arm.h:929: class ScopedConstPoolBlocker { I think we usually call these classes something ending in Scope: NoHand

[v8-dev] [v8] r4455 committed - Fix incorrect handling of global RegExp properties for nested replace-...

2010-04-21 Thread codesite-noreply
Revision: 4455 Author: l...@chromium.org Date: Wed Apr 21 01:33:04 2010 Log: Fix incorrect handling of global RegExp properties for nested replace-regexp-with-function. Review URL: http://codereview.chromium.org/1695002 http://code.google.com/p/v8/source/detail?r=4455 Modified: /branches/ble

[v8-dev] [v8] r4454 committed - Tagging version 2.2.4

2010-04-21 Thread codesite-noreply
Revision: 4454 Author: ri...@chromium.org Date: Wed Apr 21 01:29:39 2010 Log: Tagging version 2.2.4 http://code.google.com/p/v8/source/detail?r=4454 Added: /tags/2.2.4 -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev

[v8-dev] Re: Fix incorrect handling of global RegExp properties for nested replace-regexp-with-function. (issue1695002)

2010-04-21 Thread sgjesse
LGTM http://codereview.chromium.org/1695002/show -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev

[v8-dev] Fix incorrect handling of global RegExp properties for nested replace-regexp-with-function. (issue1695002)

2010-04-21 Thread lrn
Reviewers: sgjesse, Message: Smallish bugfix. Description: Fix incorrect handling of global RegExp properties for nested replace-regexp-with-function. Please review this at http://codereview.chromium.org/1695002/show Affected files: M src/regexp.js M src/string.js M test/mjsunit/string-r

[v8-dev] Re: Return the correct statement position.... (issue1752001)

2010-04-21 Thread ager
LGTM http://codereview.chromium.org/1752001/show -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev

[v8-dev] Re: Don't share function result caches between contexts. (issue1731002)

2010-04-21 Thread lrn
Drive-by comment: The cache should definitely be per-context (and I should have noticed during review. My bad!). The result of a function that belongs to one context must not be returned by a function in another context, since it might be using completely different objects to build the result

[v8-dev] [v8] r4452 committed - Return the correct statement position....

2010-04-21 Thread codesite-noreply
Revision: 4452 Author: sgje...@chromium.org Date: Wed Apr 21 00:32:04 2010 Log: Return the correct statement position. Whether this was a typo or a deliberate decision at some point I don't know. Anyway it was wrong. tbr=erik.co...@gmail.com Review URL: http://codereview.chromium.org/1752001

[v8-dev] Return the correct statement position.... (issue1752001)

2010-04-21 Thread sgjesse
Reviewers: Erik Corry, Description: Return the correct statement position. Whether this was a typo or a deliberate decision at some point I don't know. Anyway it was wrong. tbr=erik.co...@gmail.com Please review this at http://codereview.chromium.org/1752001/show SVN Base: http://v8.googlecod

[v8-dev] Re: Fix one off error. (issue1709001)

2010-04-21 Thread lrn
LGTM. Am I correct that the bug means that you write one entry (two elements) past the end of the FixedArray? http://codereview.chromium.org/1709001/diff/1/2 File src/runtime.cc (right): http://codereview.chromium.org/1709001/diff/1/2#newcode10094 src/runtime.cc:10094: if (size < cache->leng

[v8-dev] [v8] r4451 committed - Prepare push of version 2.2.4 to trunk....

2010-04-21 Thread codesite-noreply
Revision: 4451 Author: ri...@chromium.org Date: Wed Apr 21 00:10:20 2010 Log: Prepare push of version 2.2.4 to trunk. Review URL: http://codereview.chromium.org/1687004 http://code.google.com/p/v8/source/detail?r=4451 Modified: /branches/bleeding_edge/ChangeLog /branches/bleeding_edge/src/versi

[v8-dev] Re: Prepare push of version 2.2.4 to trunk. (issue1687004)

2010-04-21 Thread ricow
http://codereview.chromium.org/1687004/diff/1/3 File ChangeLog (right): http://codereview.chromium.org/1687004/diff/1/3#newcode5 ChangeLog:5: Load ICs for nonexistent properties, fixing issue 675. On 2010/04/21 07:03:37, Mads Ager wrote: I would remove this. Issue 675 was not in the latest ver

[v8-dev] Re: Prepare push of version 2.2.4 to trunk. (issue1687004)

2010-04-21 Thread ager
LGTM http://codereview.chromium.org/1687004/diff/1/3 File ChangeLog (right): http://codereview.chromium.org/1687004/diff/1/3#newcode5 ChangeLog:5: Load ICs for nonexistent properties, fixing issue 675. I would remove this. Issue 675 was not in the latest version so the new Load ICs is just a p