Reviewers: Sven Panne,
Description:
ic: perform interceptor query in LookupForWrite
Right now, v8 in a strict mode will give up setting a property on global
object if it has setter interceptor. Correct behaviour would be calling
query
interceptor or getter, if present, to figure out if the pro
On 2013/09/23 13:09:59, Toon Verwaest wrote:
It's not entirely clear to me what problem you are fixing. Can we have a
test-case for this?
Previously if there was an interceptor setter, we'd always initialize the
IC
because lookup->CanHoldValue(value) would always return true (given that
repr
On 2013/09/23 13:20:13, Toon Verwaest wrote:
Done 14 minutes ago ;) https://codereview.chromium.org/23480102/
Great! :)
Seems to be working for me in node.js, thank you for letting me know and
looking
at this CL.
https://codereview.chromium.org/24272005/
--
--
v8-dev mailing list
v8-dev@g
Base URL is incorrect, please update it if it's possible:
https://github.com/indutny/v8/tree/feature-switch-string-literal-opt
http://codereview.chromium.org/8373029/
--
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
Reviewers: Vyacheslav Egorov,
Description:
[hydrogen] optimize switch with string clauses
Hydrogen should optimize not only SMI clauses, but clauses with string
literals
too.
R=vego...@chromium.org
BUG=
TEST=
Please review this at http://codereview.chromium.org/8373029/
SVN Base: gh:v8/v8
Updated.
http://codereview.chromium.org/8373029/
--
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
Updated
http://codereview.chromium.org/8373029/
--
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
Updated
http://codereview.chromium.org/8373029/
--
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
Can you please benchmark this branch. It's faster on jslint for me.
http://codereview.chromium.org/8373029/
--
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
Finally, this thing works w/o segfaults and assertions.
On my machine jslint is about 5% faster on patched d8 than on bleeding_edge.
Added x64 instruction, but can't test it (only have 32bit mbpro right now).
http://codereview.chromium.org/8373029/
--
v8-dev mailing list
v8-dev@googlegroups.com
http://codereview.chromium.org/8373029/diff/32026/src/ast.h
File src/ast.h (right):
http://codereview.chromium.org/8373029/diff/32026/src/ast.h#newcode713
src/ast.h:713: bool IsSymbolCompare() { return compare_type_ ==
SYMBOL_ONLY; }
On 2011/11/01 09:26:19, fschneider wrote:
You introduce IsSym
btw, it fails sometimes when trying to apply uglifyjs on itself, and I don't
understand why (as it works in everything from jslint to my own custom apps)
http://codereview.chromium.org/8373029/
--
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
Any news?
http://codereview.chromium.org/8373029/
--
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
As I can see, problem comes from different place. `advance` was bailout'ed
because of switch clause type restrictions, my patch is removing those
restrictions and it's starting deoptimization on some smi checks (as I can
see)
that wasn't introduced by my patch.
Even more, on previous revisions
So I guess that performance regression was caused by some v8 team's commits
and
we should consider fixing that or at least reverting regression commit.
http://codereview.chromium.org/8373029/
--
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
Renamed oddball_check.
Added type feedback check for bailouting switches that are taking non-symbol
strings as input.
http://codereview.chromium.org/8373029/
--
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
Any updates?
http://codereview.chromium.org/8373029/
--
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
http://codereview.chromium.org/8373029/diff/43002/src/hydrogen.cc
File src/hydrogen.cc (right):
http://codereview.chromium.org/8373029/diff/43002/src/hydrogen.cc#newcode2762
src/hydrogen.cc:2762: iterations = iterations * 2;
It yields a big improvement, as we usually don't know any type feedback
http://codereview.chromium.org/8373029/diff/43002/src/ia32/lithium-codegen-ia32.cc
File src/ia32/lithium-codegen-ia32.cc (right):
http://codereview.chromium.org/8373029/diff/43002/src/ia32/lithium-codegen-ia32.cc#newcode1798
src/ia32/lithium-codegen-ia32.cc:1798: Handle ic =
CompareIC::GetUninit
http://codereview.chromium.org/8373029/diff/43002/src/ia32/lithium-ia32.cc
File src/ia32/lithium-ia32.cc (right):
http://codereview.chromium.org/8373029/diff/43002/src/ia32/lithium-ia32.cc#newcode1548
src/ia32/lithium-ia32.cc:1548: return
AssignEnvironment(MarkAsCall(result, instr));
You're righ
Fixed all things you commented. ;)
http://codereview.chromium.org/8373029/
--
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
http://codereview.chromium.org/8373029/diff/43002/src/hydrogen.cc
File src/hydrogen.cc (right):
http://codereview.chromium.org/8373029/diff/43002/src/hydrogen.cc#newcode2762
src/hydrogen.cc:2762: iterations = iterations * 2;
I tried implementing graph structure like that:
|
/\
/
Thanks for reviewing it! Should I open a new request with rest of the
features
after you land it?
http://codereview.chromium.org/8373029/
--
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
Of course, running tests!
Cheers,
Fedor.
On Tue, Nov 8, 2011 at 3:31 PM, Florian Schneider
wrote:
> Unfortunately x64 shows a test failure in the mozilla test suite:
>
>
> http://build.chromium.org/p/client.v8/builders/V8%20Linux64/builds/2863/steps/Mozilla/logs/stdio
>
>
Fixed, thanks
http://codereview.chromium.org/8373029/
--
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
fixed, thanks
http://codereview.chromium.org/8373029/
--
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
Agreed.
Is it blocking landing of current version of patch?
Cheers,
Fedor.
On Tue, Nov 8, 2011 at 6:07 PM, Vyacheslav Egorov wrote:
> We need to increase the coverage of the switch test for different
> switch use cases (string, smi, mixed), different inputs (symbol
> strings, n
(empty message, just to remind you about this issue)
http://codereview.chromium.org/8373029/
--
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
Added comprehensive test for different type of clauses, tags, feedback and
optimizations.
http://codereview.chromium.org/8373029/
--
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
Thanks, fixed
http://codereview.chromium.org/8373029/
--
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
Thank you for merging this!
I opened new code review request to add second-pass of `fast` symbol
comparisons and some type feedback to this switch optimization:
http://codereview.chromium.org/8499010
Cheers,
Fedor.
On Wed, Nov 9, 2011 at 12:41 PM, wrote:
> (empty message, just to rem
Any status updates on http://codereview.chromium.org/8499010/ ?
Cheers,
Fedor.
On Fri, Nov 18, 2011 at 12:07 PM, Fedor Indutny wrote:
> Thank you for merging this!
>
> I opened new code review request to add second-pass of `fast` symbol
> comparisons and some type feedback to
Reviewers: Vyacheslav Egorov, fschneider,
Message:
Everything fixed, and I noticed that it was anyway bailouting on
self-referencing named function, because of VisitVariableProxy for CONST
variable in CONTEXT. Do we need this bailout anymore?
And looks like my test is failing on new test: "illeg
Reviewers: Vyacheslav Egorov,
Description:
[debugger] noRefs option for fixing blowing array refs
see https://gist.github.com/1230388 for example. When requesting bactrace -
node.js SlowBuffer appears as an argument and it's every index is being put
to
the 'ref' property.
In this patch I int
Sorry, bad grammar, never write replies too late :) hope everything is clear
from my message anyway.
http://codereview.chromium.org/8857001/
--
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
Updated with fixes
http://codereview.chromium.org/8857001/
--
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
Not right now, but going to investigate...
http://codereview.chromium.org/8857001/
--
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
Fixed, I was checking value, not target in StoreContextSlot.
http://codereview.chromium.org/8857001/
--
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
any status update?
http://codereview.chromium.org/8873053/
--
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
any status update?
http://codereview.chromium.org/8857001/
--
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
Done!
http://codereview.chromium.org/8857001/
--
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
Done.
http://codereview.chromium.org/8857001/diff/17002/src/hydrogen-instructions.h
File src/hydrogen-instructions.h (right):
http://codereview.chromium.org/8857001/diff/17002/src/hydrogen-instructions.h#newcode3549
src/hydrogen-instructions.h:3549: return mode_ ==
kCheckIgnoreAssignment;
oh, s
Done
http://codereview.chromium.org/8857001/
--
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
No, it can't as you can see in gist that I attached to this patch, in
backtrace
request refs will mirror all arguments in all scopes, and that's ok in most
cases. But when you passing TypedArrays or Buffers to callbacks, each their
element will be mirrored and put into ref.
But we don't need to
http://codereview.chromium.org/8873053/diff/7002/src/debug-debugger.js
File src/debug-debugger.js (right):
http://codereview.chromium.org/8873053/diff/7002/src/debug-debugger.js#newcode1352
src/debug-debugger.js:1352: if (!this.options_ || !this.options_.noRefs)
{
You mean includeRefs: true, rig
Oh, I made a typo : includeRefs => inlineRefs.
And I've tried using it and it works
http://codereview.chromium.org/8873053/
--
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
Reviewers: Erik Corry,
Description:
[objects] seed NumberDictionary (only ia32) now
BUG=
TEST=
R=erik.co...@gmail.com
Please review this at http://codereview.chromium.org/9148006/
SVN Base: gh:v8/v8@master
Affected files:
M src/ia32/macro-assembler-ia32.cc
M src/objects-inl.h
M src/ob
sorry, patch is incomplete/incorrect now, fixing it right now
http://codereview.chromium.org/9148006/
--
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
I think it works now, can you please review?
http://codereview.chromium.org/9148006/
--
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
I can add const bool variable UsesSeed to all shapes.
And check it before calling GetHeap()->StringHashSeed() , that'll
definitely be
optimized by most of C++ compilers (unused arguments are optimized only by
g++,
as bnoordhuis told me).
What do you think?
http://codereview.chromium.org/914
updated commit request.
Removed all useless arguments, added const bool variable and SeededHash
method
to shape's base class (which was introduced here, btw)
If design is ok, I can start porting it to other archs.
http://codereview.chromium.org/9148006/diff/1011/src/utils.h
File src/utils.h
Should be fixed now. Tested in debug and release mode with snapshots and
w/o.
http://codereview.chromium.org/9148006/
--
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
Added test, fixed code, decoupled things :)
http://codereview.chromium.org/9148006/
--
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
rebased
http://codereview.chromium.org/9148006/
--
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
Fixed issues, fixed rebase
http://codereview.chromium.org/9148006/
--
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
Reviewers: Erik Corry,
Description:
Backport @10366 to 3.6
Add seed to hash of numeric keyed properties.
R=erik.co...@gmail.com
Please review this at http://codereview.chromium.org/9190001/
SVN Base: http://v8.googlecode.com/svn/branches/3.6/
Affected files:
M src/arm/macro-assembler-a
Reviewers: Erik Corry,
Message:
It's causing segfaults currently, hope you can guide me or use parts of it
code.
Description:
[objects] NotSeededNumberDictionary
BUG=
TEST=
R=erik.co...@gmail.com
Please review this at http://codereview.chromium.org/9212006/
SVN Base: gh:v8/v8@master
Aff
Reviewers: Erik Corry,
Description:
objects: fix linker problem
R=erik.co...@gmail.com
BUG=1936
TEST=
Please review this at http://codereview.chromium.org/9382033/
SVN Base: gh:v8/v8@master
Affected files:
M src/objects.cc
Index: src/objects.cc
diff --git a/src/objects.cc b/src/objects.
On 2012/02/14 09:41:34, danno wrote:
LGTM. Landing.
Can we backport it to 3.6.x?
https://chromiumcodereview.appspot.com/9382033/
--
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
The error I reported in issue was noticed on node.js v0.6.x which uses 3.6.
Sorry, but I can't tell you more about it.
On 2012/02/14 12:06:15, danno wrote:
Is this the only symbol that needs to be defined for the backport? Erik
added
about 20 new definitions for 3.8 for Seeded and Unseeded dict
Ok, Thank you anyway!
https://chromiumcodereview.appspot.com/9382033/
--
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
How to use StackFrames from TryCatch::StackTrace?
Any help/hints will be usefull!
Thank you!
--
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
Reviewers: danno,
Message:
Probable improvements and things to done:
* Improve resetting counters by dictionary of references to type feedback
cells
* Handle counter overflow (isn't actually needed if below will be done)
* Remove counters from unoptimized code once function is marked as
unopt
On 2013/06/02 08:21:15, indutny wrote:
Probable improvements and things to done:
* Improve resetting counters by dictionary of references to type feedback
cells
* Handle counter overflow (isn't actually needed if below will be done)
* Remove counters from unoptimized code once function is ma
On 2013/06/02 09:12:48, indutny wrote:
On 2013/06/02 08:21:15, indutny wrote:
> Probable improvements and things to done:
>
> * Improve resetting counters by dictionary of references to type
feedback
cells
> * Handle counter overflow (isn't actually needed if below will be done)
> * Remove cou
On 2013/06/02 09:21:37, indutny wrote:
On 2013/06/02 09:12:48, indutny wrote:
> On 2013/06/02 08:21:15, indutny wrote:
> > Probable improvements and things to done:
> >
> > * Improve resetting counters by dictionary of references to type
feedback
> cells
> > * Handle counter overflow (isn't ac
On 2013/06/02 20:57:26, indutny wrote:
On 2013/06/02 09:21:37, indutny wrote:
> On 2013/06/02 09:12:48, indutny wrote:
> > On 2013/06/02 08:21:15, indutny wrote:
> > > Probable improvements and things to done:
> > >
> > > * Improve resetting counters by dictionary of references to type
feedback
Just FYI to consider, benchmarks seems to be running a little bit faster
after
applying this patch.
Before:
Richards: 15995
DeltaBlue: 17270
Crypto: 17093
RayTrace: 17763
EarleyBoyer: 20444
RegExp: 2822
Splay: 5866
NavierStokes: 20310
Score (version 7): 12448
After:
Richards: 15992
Delt
Thank you for reviewing, sad that it won't get in, but that's ok. I'll look
into
proposed optimization techniques and will share results with you (if I'll
have
any).
https://codereview.chromium.org/16128004/diff/22001/src/hydrogen-instructions.h
File src/hydrogen-instructions.h (right):
htt
execution - deoptimization will happen only after
the counter value will be exhausted, which should take reasonable amount of
time, considering rareness.
Please let me know if you want me to port this code to use HLoad, HStore or
anything else.
Cheers,
Fedor.
On Mon, Jun 3, 2013 at 3:42 PM, Sven Pa
Btw, why are we discussing it here and not on CL page?
Cheers,
Fedor.
On Mon, Jun 3, 2013 at 3:46 PM, Fedor Indutny wrote:
> Sven,
>
> I think my deoptimization technique could be greatly improved, but even
> now it works perfectly for supplied test case as it recompiles
Deoptimizing on unvisited cases will not improve anything, it'll just cause
spontaneous deoptimizations. If you'll look at my CL history, I tried this
solution in first place before going into counter cell wilderness.
Cheers,
Fedor.
On Mon, Jun 3, 2013 at 6:25 PM, Florian Schne
On 2013/06/03 10:56:11, indutny wrote:
Thank you for reviewing, sad that it won't get in, but that's ok. I'll
look
into
proposed optimization techniques and will share results with you (if I'll
have
any).
https://codereview.chromium.org/16128004/diff/22001/src/hydrogen-instructions.h
Fil
On 2013/06/03 19:37:18, indutny wrote:
On 2013/06/03 10:56:11, indutny wrote:
> Thank you for reviewing, sad that it won't get in, but that's ok. I'll
look
into
> proposed optimization techniques and will share results with you (if
I'll
have
> any).
>
>
https://codereview.chromium.org/16
On 2013/06/03 20:04:04, indutny wrote:
On 2013/06/03 19:37:18, indutny wrote:
> On 2013/06/03 10:56:11, indutny wrote:
> > Thank you for reviewing, sad that it won't get in, but that's ok. I'll
look
> into
> > proposed optimization techniques and will share results with you (if
I'll
have
> >
nk that we should try accumulating hit stats for
clauses.
Cheers,
Fedor.
On Tue, Jun 4, 2013 at 10:48 AM, Sven Panne wrote:
> On Mon, Jun 3, 2013 at 4:25 PM, Florian Schneider > wrote:
>
>> I'm not saying that other techniques are worse or better by any means. I
>> t
What about my patch for cell clearing code? Do you think its still worth
applying, even without main CL?
Cheers,
Fedor.
On Tue, Jun 4, 2013 at 11:13 AM, Sven Panne wrote:
> On Tue, Jun 4, 2013 at 8:53 AM, Fedor Indutny wrote:
>
>> Sorry, I know that's the normal way of do
Reviewers: Sven Panne,
Description:
hydrogen: split switch optimization code into multiple functions
In preparions for future switch statement optimization improvements.
R=svenpa...@chromium.org
BUG=
Please review this at https://codereview.chromium.org/15993014/
SVN Base: gh:v8/v8.git@maste
ike a good idea to
you?
Cheers,
Fedor.
On Tue, Jun 4, 2013 at 1:21 PM, Fedor Indutny wrote:
> What about my patch for cell clearing code? Do you think its still worth
> applying, even without main CL?
>
> Cheers,
> Fedor.
>
>
> On Tue, Jun 4, 2013 at 11:13 AM, Sven Pan
Ok, I'll wait for you guys to plan it.
Cheers,
Fedor.
On Wed, Jun 5, 2013 at 11:35 AM, Sven Panne wrote:
> On Wed, Jun 5, 2013 at 8:47 AM, Sven Panne wrote:
>
>> On Tue, Jun 4, 2013 at 6:02 PM, Fedor Indutny wrote:
>>
>>> As a start for all this stuff that
itive code, or at least I don't know any public example of their usage.
Cheers,
Fedor.
On Wed, Jun 5, 2013 at 12:57 PM, Fedor Indutny wrote:
> Ok, I'll wait for you guys to plan it.
>
> Cheers,
> Fedor.
>
>
> On Wed, Jun 5, 2013 at 11:35 AM, Sven Panne wrote:
Reviewers: Erik Corry,
Message:
Note that it is a Work-In-Progress, and lacks implementation of `Has()`
method
so far.
Description:
api: Object::CachedProperty
This is a work in progress, just want to share it with you guys to get some
critics and comments about it!
From my benchmarks prope
On 2013/04/28 21:08:28, indutny wrote:
Note that it is a Work-In-Progress, and lacks implementation of `Has()`
method
so far.
Ok, I decided to avoid implementing `Has()`.
https://codereview.chromium.org/14425011/
--
--
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/gro
On 2013/04/28 21:15:11, indutny wrote:
On 2013/04/28 21:08:28, indutny wrote:
> Note that it is a Work-In-Progress, and lacks implementation of `Has()`
method
> so far.
Ok, I decided to avoid implementing `Has()`.
It seems to be ready for review, I'll do benchmarks with node.js soon.
http
On 2013/04/28 21:51:33, indutny wrote:
On 2013/04/28 21:15:11, indutny wrote:
> On 2013/04/28 21:08:28, indutny wrote:
> > Note that it is a Work-In-Progress, and lacks implementation of
`Has()`
method
> > so far.
>
> Ok, I decided to avoid implementing `Has()`.
It seems to be ready for rev
On 2013/04/28 22:30:57, indutny wrote:
On 2013/04/28 21:51:33, indutny wrote:
> On 2013/04/28 21:15:11, indutny wrote:
> > On 2013/04/28 21:08:28, indutny wrote:
> > > Note that it is a Work-In-Progress, and lacks implementation of
`Has()`
> method
> > > so far.
> >
> > Ok, I decided to avoid
On 2013/04/29 11:30:11, danno wrote:
Fedor, I have some concerns about modifying the API like this (in
addition to
the fundamental implementation issue below). It pollutes the existing
Get/Set
APIs and adds a fair amount of duplication for an arguably non-core use
case.
If
performance is
87 matches
Mail list logo
