LGTM
http://codereview.chromium.org/3436006/show
--
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
LGTM, good catch
Whether you want to move the checking to
SharedFunctionInfo::SetThisPropertyAssignmentsInfo is up to you.
http://codereview.chromium.org/3434004/diff/1/2
File src/heap.cc (right):
http://codereview.chromium.org/3434004/diff/1/2#newcode2717
src/heap.cc:2717: // guarantee the un
LGTM, but why are all the method implementations moved from
macro-assembler-x64.cc to the header file?
http://codereview.chromium.org/3381005/diff/1/5
File src/x64/macro-assembler-x64.h (right):
http://codereview.chromium.org/3381005/diff/1/5#newcode1017
src/x64/macro-assembler-x64.h:1017: ASSE
Could you please take another look?
The change now actually improve splay.js (this is the test I've used for
profiling). Here are changes:
Page gets a new member in the header, "heap". This does not increase the
size of
the header since it is 32-byte aligned (on ia32).
The ScavengeVisitor al
Reviewers: Søren Gjesse,
Description:
Enhance SafeStackFrameIterator to avoid triggering assertions in debug mode.
When running profiling in debug mode, several assertions in frame
iterators that are undoubtedly useful when iterator is started from a
VM thread in a known "good" state, may fail w
Revision: 5464
Author: yu...@chromium.org
Date: Wed Sep 15 08:34:43 2010
Log: [Isolates] Make isolate tests pass on x64
* Instead of calling Space::identity() on deleted Space object
store a pointer to the identity in the ChunkInfo so that it
can be accessed even if the space has been destroy
LGTM
http://codereview.chromium.org/3357023/show
--
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
Revision: 5463
Author: yu...@chromium.org
Date: Wed Sep 15 08:20:49 2010
Log: Pass current isolate address to the regexp native functions. Immediate
effect of this change is a modest speed up(<4%)on a few tests but we will
need isolate pointer to pass it directly into Handle constructors and
Isolate* argument on ARM is passed as was proposed by vitalyr who introduced
CallCFunctionHelper in macro-assembler-arm.* Thanks for your advice Vitaly.
http://codereview.chromium.org/3357023/show
--
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
Drive-by: when you agree on the patch, please merge to 2.2 and 2.3
branches. :)
http://codereview.chromium.org/3434004/show
--
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
Reviewers: Søren Gjesse,
Description:
Prevent inline constructor generation when duplicate properties are present
in
the constructor.
Currenly the constructor like this:
function f() {
this.a = 0;
this.a = 1;
this.a = 2;
}
creates a map with duplicate desciptors which is bad in many way
Reviewers: Rico,
Description:
X64: Templating Smi-macros to use both Label and NearLabel.
Added some more uses of NearLabel.
Please review this at http://codereview.chromium.org/3381005/show
Affected files:
M src/x64/code-stubs-x64.cc
M src/x64/ic-x64.cc
M src/x64/macro-assembler-x64.h
Dears:
I want to terminate the javascript running using
TerminateExecution(), but I find the javascript is still running in
the v8 thread, so when I try to reset context() it will crash
sometimes.
How can I check the javascript it not running in V8 thread after I
call TerminateExecution()?
LGTM
http://codereview.chromium.org/3421009/show
--
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
Revision: 5461
Author: erik.co...@gmail.com
Date: Wed Sep 15 05:17:41 2010
Log: Made the use of past tense more consistent in change log.
Review URL: http://codereview.chromium.org/3425005
http://code.google.com/p/v8/source/detail?r=5461
Modified:
/branches/bleeding_edge/ChangeLog
=
Excellent. LGTM.
http://codereview.chromium.org/3425005/show
--
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
Reviewers: Kevin Millikin,
Description:
Made the use of past tense more consistent in change log.
Please review this at http://codereview.chromium.org/3425005/show
SVN Base: http://v8.googlecode.com/svn/branches/bleeding_edge/
Affected files:
M ChangeLog
--
v8-dev mailing list
v8-dev@g
Revision: 5460
Author: ri...@chromium.org
Date: Wed Sep 15 04:43:12 2010
Log: Add support for near labels.
This change introduces near labels in the assembler, allowing us to
uptimize forward jumps (conditional and unconditional) if we can
guarantee that the jump is witin range -128 to +127.
I c
Some code depends on ip staying the same after native calls. I found that
e.g.
GenericBinaryOpStub::HandleBinaryOpSlowCases would break if I used ip as a
scratch register in CallCFunction. As an alternative we could try to pass a
scratch register as an argument to CallCFunction but that would di
Commit messages normally have blank lines separating the lines of text.
On Wed, Sep 15, 2010 at 1:28 PM, wrote:
> Reviewers: Mads Ager,
>
> Description:
> Revision 2.4.4.
> Fix bug with hangs on very large sparse arrays.
> Try harder to free up memory when running out of space.
> Add heap snapsh
Reviewers: Mads Ager,
Description:
Revision 2.4.4.
Fix bug with hangs on very large sparse arrays.
Try harder to free up memory when running out of space.
Add heap snapshots to JSON format to API.
Recalibrate benchmarks.
Please review this at http://codereview.chromium.org/3421009/show
SVN Base
I know it's a losing battle, but the ChangeLog entries are normally past
tense, if only for consistency with 2+ years of the project.
On Wed, Sep 15, 2010 at 12:54 PM, wrote:
> LGTM
>
>
> http://codereview.chromium.org/3429006/show
>
> --
> v8-dev mailing list
> v8-dev@googlegroups.com
> http://
I will refactor the NearLabel class in another change.
http://codereview.chromium.org/3388004/diff/1/10
File src/x64/full-codegen-x64.cc (right):
http://codereview.chromium.org/3388004/diff/1/10#newcode681
src/x64/full-codegen-x64.cc:681: Label slow_case;
On 2010/09/15 11:21:08, fschneider wrot
LGTM.
http://codereview.chromium.org/3388004/diff/1/10
File src/x64/full-codegen-x64.cc (right):
http://codereview.chromium.org/3388004/diff/1/10#newcode681
src/x64/full-codegen-x64.cc:681: Label slow_case;
Indentation off. I think this can be a NearLabel too.
http://codereview.chromium.org/33
Revision: 5459
Author: erik.co...@gmail.com
Date: Wed Sep 15 03:58:25 2010
Log: Prepare push to trunk. We are now working on version 2.4.5.
Review URL: http://codereview.chromium.org/3429006
http://code.google.com/p/v8/source/detail?r=5459
Modified:
/branches/bleeding_edge/ChangeLog
/branches/
Revision: 5458
Author: l...@chromium.org
Date: Wed Sep 15 03:54:35 2010
Log: Made predata smaller by storing symbol data in variable length
base-128.
Remove position from symbol data - they must come in the correct order
anyway.
Review URL: http://codereview.chromium.org/3384003
http://code.
LGTM
http://codereview.chromium.org/3429006/show
--
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
http://codereview.chromium.org/3384003/diff/1/2
File src/parser.cc (right):
http://codereview.chromium.org/3384003/diff/1/2#newcode1663
src/parser.cc:1663: symbol_id =
Done.
http://codereview.chromium.org/3384003/diff/1/2#newcode5514
src/parser.cc:5514: symbol_data_ =
Done.
http://codereview.c
Reviewers: Mads Ager,
Description:
Prepare push to trunk. We are now working on version 2.4.5.
Please review this at http://codereview.chromium.org/3429006/show
SVN Base: http://v8.googlecode.com/svn/branches/bleeding_edge/
Affected files:
M ChangeLog
M src/version.cc
Index: Cha
Drive by comment
http://codereview.chromium.org/3388004/diff/1/2
File src/assembler.h (right):
http://codereview.chromium.org/3388004/diff/1/2#newcode100
src/assembler.h:100: class NearLabel BASE_EMBEDDED {
How about adding a type to the Label class
class Label ...
enum LabelType {
kTyp
On Wed, Sep 15, 2010 at 11:05 AM, Vitaly Repeshko wrote:
> On Wed, Sep 15, 2010 at 11:33 AM, Mads Sig Ager wrote:
>> Sounds like a good plan to me! :)
>>
>> I don't think I understand the part about "by the time we decide to
>> resize the map there might be no live objects created from this
>> pa
Committed as bleeding edge revision 5457. Thankyou.
http://codereview.chromium.org/3341012/diff/2002/18002
File src/arm/code-stubs-arm.cc (left):
http://codereview.chromium.org/3341012/diff/2002/18002#oldcode4169
src/arm/code-stubs-arm.cc:4169: // r7: to (smi)
I put these comments back.
http:
Revision: 5457
Author: erik.co...@gmail.com
Date: Wed Sep 15 03:22:55 2010
Log: Replace 2 ARM ldr instructions with one ldrd in the code generated
for a SubStringStub and StringCompareStub in the ARM backend. This
is a commit of http://codereview.chromium.org/3341012 for Andreas
Anyuru.
Review UR
http://codereview.chromium.org/3387003/diff/1/3
File src/arm/code-stubs-arm.cc (right):
http://codereview.chromium.org/3387003/diff/1/3#newcode4218
src/arm/code-stubs-arm.cc:4218: __ b(lt, &runtime); // Fail if to >
length.
On 2010/09/15 10:09:07, Søren Gjesse wrote:
Looks as if "to" is not li
Revision: 5456
Author: fschnei...@chromium.org
Date: Wed Sep 15 03:14:25 2010
Log: Make the CompareStub and the UnaryOpStub accept smi inputs.
The stubs get an additional flag for including the smi code
inside the stub. This allows us to generate more compact code
if we don't want to inline the s
LGTM
http://codereview.chromium.org/3387003/diff/1/3
File src/arm/code-stubs-arm.cc (right):
http://codereview.chromium.org/3387003/diff/1/3#newcode4218
src/arm/code-stubs-arm.cc:4218: __ b(lt, &runtime); // Fail if to >
length.
Looks as if "to" is not live after here, maybe set it to no_reg a
Reviewers: fschneider,
Description:
Add support for near labels.
This change introduces near labels in the assembler, allowing us to
uptimize forward jumps (conditional and unconditional) if we can
guarantee that the jump is witin range -128 to +127.
I changed a large fractions of the existing
Reviewers: Søren Gjesse,
Description:
Replace 2 ARM ldr instructions with one ldrd in the code generated
for a SubStringStub and StringCompareStub in the ARM backend. This
is a commit of http://codereview.chromium.org/3341012 for Andreas
Anyuru.
Please review this at http://codereview.chromium.
LGTM.
http://codereview.chromium.org/3388005/diff/14001/13008
File src/flag-definitions.h (right):
http://codereview.chromium.org/3388005/diff/14001/13008#newcode177
src/flag-definitions.h:177: // full-codegen.cc / full-codegen-ia32.cc
Remove the ia32.cc reference.
http://codereview.chromium.o
Reviewers: Kasper Lund, Erik Corry,
Message:
This is a new version where I added the x64 and ARM parts.
Description:
Make the CompareStub and the UnaryOpStub accept smi inputs.
The stubs get an additional flag for including the smi code
inside the stub. This allows us to generate more compact c
On Wed, Sep 15, 2010 at 11:33 AM, Mads Sig Ager wrote:
> Sounds like a good plan to me! :)
>
> I don't think I understand the part about "by the time we decide to
> resize the map there might be no live objects created from this
> particular map". Does that matter? It seems to me that it would sti
This approach seems good. Comments below.
http://codereview.chromium.org/3449004/diff/1/2
File src/ast.h (right):
http://codereview.chromium.org/3449004/diff/1/2#newcode181
src/ast.h:181: // Evaluated for its value (and side effects). Result on
stack.
The distinction between Stack/Accumulator
LGTM
http://codereview.chromium.org/3382007/show
--
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
This is work in progress, but I don't want to waste any more time if the
approach is not right. The main file I want you to look at is
full-codegen-ia32.cc. The other files still have stuff that needs cleaning
up,
but I want to be careful about the order I do it in so as not to forego the
pos
Reviewers: Kevin Millikin,
Description:
Cleanup of contexts in the full code generator.
Please review this at http://codereview.chromium.org/3449004/show
SVN Base: http://v8.googlecode.com/svn/branches/bleeding_edge/
Affected files:
M src/ast.h
M src/full-codegen.h
M src/full
Sounds like a good plan to me! :)
I don't think I understand the part about "by the time we decide to
resize the map there might be no live objects created from this
particular map". Does that matter? It seems to me that it would still
be a good idea to update the inobject-property count?
Cheers,
LGTM
http://codereview.chromium.org/3384003/diff/1/2
File src/parser.cc (right):
http://codereview.chromium.org/3384003/diff/1/2#newcode1663
src/parser.cc:1663: symbol_id =
Will fit on one line now.
http://codereview.chromium.org/3384003/diff/1/2#newcode5514
src/parser.cc:5514: symbol_data_ =
47 matches
Mail list logo
