654190 - Special Integer String Type

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Tom S [:evilpie]]

I think v8 has this, and Opera maybe, too. From what i read somewhere, when looping over an array in v8 using for..in, a special kind of strings is created, only containing the integer for faster key access.

for (var x in (y = [1, 2, 3, 4, 5]))
   y[x]

Comment 1

[Luke Wagner [:luke]]

That is a great idea.  This should fit into the string hierarchy without too much hassle: just another type that derives JSString and gets flattened if necessary.  We are a bit tight on flag bits, so the only downside is we may have to deoptimize some existing tests.

Comment 2

[Tim Taubert [:ttaubert] (inactive)]

This looks like a good first bug to me. Do you have some hints or pointers to make the start a little bit easier? Thanks :)

Comment 3

[Eddy Bruel [:ejpbruel]]

I would also like to work on this bug. Tim, are you still planning to work on this?

Comment 4

[Luke Wagner [:luke]]

I'll let you two decide who takes the bug; hopefully the matter won't be settled 18th-century-mathematician-style :)

A good starting point is the big comment atop js/src/vm/String.h and then the slightly smaller encoding comment right below.  This should give you the idea of how we handle specialized string types and where the fancy new "JSIntString" would fit in.

But first, I think we should understand where this optimization would matter and measure how often it would hit for real web sites and benchmarks (viz. SunSpider, V8, and Kraken) lest we add complexity without speeding anything up.  I think the easiest way to do this measurement would be to (temporarily) add a 'bool has_int' field to JSString and set this to 'true' when you know you can create one of these new int-strings.  Now, just creating an int-string isn't enough (it doesn't save char-buffer-malloc since JSShortString can hold the chars of any int inline); we need to use it *as an int-string* somewhere.  So the other half of the measurement is to find the places which could consume an int-string and count how many strings pass through with has_int == true (as an absolute count and as % of all strings passing through).  Comment 0 is a good example of this.  I bet if you grep through v8's source code you could find more :)

I hope that helps, let me know if you have any questions.

Comment 5

[Tom S [:evilpie]]

How is this going? I could pick this up if you don't wan to do it anymore.

Comment 6

[Eddy Bruel [:ejpbruel]]

Since both me and Tim volunteered for this bug, I talked to him a while ago on IRC and we agreed he would work on it (since he volunteered first). If for any reason he doesn't want to work on it anymore I would still like to do it (unless Tom really *really* wants it).

Comment 7

[Tom S [:evilpie]]

(In reply to ejpbruel from comment #6)
> Since both me and Tim volunteered for this bug, I talked to him a while ago
> on IRC and we agreed he would work on it (since he volunteered first). If
> for any reason he doesn't want to work on it anymore I would still like to
> do it (unless Tom really *really* wants it).
No, i am happy when you do it.

Comment 8

[Tim Taubert [:ttaubert] (inactive)]

Alas, I didn't find enough time to continue this. It also turned out not to be the easy fix I thought it would be to start with SM development. Don't want to block anyone so have fun, Eddy :)

Comment 9

[Eddy Bruel [:ejpbruel]]

Awesome! I still have a couple of bugs on my backlog, so I'll start working on this one asap.

Comment 10

[Eddy Bruel [:ejpbruel]]

As a first step, we need to introduce an instance encoding for integer based strings. Logically, JSIntString should inherit from JSString (it does not have a chars representation, after all). I propose using xx01 to indicate ropes, and xx11 to indicate integer based strings. This way, linear strings and everything inherited from them can keep the same instance encoding. The only caveat is that dependent strings have to be checked against xx10 instead of just xx1x, to avoid confusion with integer based strings.

Luke, I would like your feedback on if this encoding makes sense to you. If it does, I'll proceed with implementing it (specifically, isRope() and isDependent() will have to be changed, as does all code that sets DEPENDENT_BIT).

Comment 11

[Luke Wagner [:luke]]

(In reply to ejpbruel from comment #10)
Sounds reasonable to me!  Don't forget LINEAR_MASK/LINEAR_FLAGS :)  Now, the isLinear() test (!isRope()) is very common and technically this new encoding would take an extra op (test vs. and+test) for isLinear.  Fortunately, there is a good chance that it doesn't matter.  A good first step would be to first change the encoding (without adding the JSIntString encoding) and (after running jit-tests :) measure bef/aft on SunSpider and V8.  (Shell SS is available at http://svn.webkit.org/repository/webkit/trunk/PerformanceTests/SunSpider and shell v8 can be run from moz/js/src/v8/run.js.)

Comment 12

[Tom S [:evilpie]]

Also it would be good to figure out (different bug) when it would make sense to handle JSIntString special way. For example when using Number() or in the stubs for >>. Would be awesome if we could figure out where this would make sense. Maybe could always store this kind of stuff in integer strings, but deoptimize if it slows us down. Also i just recognized that this is going to be awesome for the JITs, because we could inline int -> string conversation.

Comment 13

[Luke Wagner [:luke]]

(In reply to Tom Schuster (evilpie) from comment #12)
Yeah, that's what I was asking about in comment 4.  I suggested to ttaubert just setting a flag on JSStrings that meant "this could be JSIntString if we had actually implemented them" and measuring that, but implementing JSIntString does sound like more fun :)

Comment 14

[Eddy Bruel [:ejpbruel]]

(In reply to Luke Wagner [:luke] from comment #11)
> (In reply to ejpbruel from comment #10)
> Sounds reasonable to me!  Don't forget LINEAR_MASK/LINEAR_FLAGS :)  Now, the
> isLinear() test (!isRope()) is very common and technically this new encoding
> would take an extra op (test vs. and+test) for isLinear.  Fortunately, there
> is a good chance that it doesn't matter.  A good first step would be to
> first change the encoding (without adding the JSIntString encoding) and
> (after running jit-tests :) measure bef/aft on SunSpider and V8.  (Shell SS
> is available at
> http://svn.webkit.org/repository/webkit/trunk/PerformanceTests/SunSpider and
> shell v8 can be run from moz/js/src/v8/run.js.)

Correct me if I'm wrong, but in the original scheme, to check if a string is linear required checking it against xxx0, since ropes have a 1 in their lsb. Since integer based strings have a 1 in their lsb as well, this check should still be valid, and nothing would have to be changed here.

Comment 15

[Eddy Bruel [:ejpbruel]]

Attachment: Refactored existing instance encoding

This patch refactors the existing instance encoding as we discussed. The next step will be to add the instance encoding for integer based strings and add a class JSIntString that will implement them.

Comment 16

[Luke Wagner [:luke]]

(In reply to ejpbruel from comment #14)
Ah, sorry, you're right!

Comment 17

[Eddy Bruel [:ejpbruel]]

Attachment: Implemented JSIntString

This patch adds the instance encoding for integer based string and the class JSIntString to implement them. I also had to make some changes in jstracer.cpp and FastBuiltins.cpp because ropes are now identified by two bits rather than one (Luke, could you please take a look at those changes and see if they make any sense?). Presumably, we also need some code to flatten strings of this type. I was thinking about using IntToCString for that.

Anywaym, next step seems to me to actually incorporate JSIntString in the engine and measure if it leads to any performance improvements. Could anybody give me some pointers on how to go about that? (Luke, I'm looking at you again).

Comment 18

[Luke Wagner [:luke]]

Comment on attachment 558293 [details] [diff] [review]
Implemented JSIntString

Review of attachment 558293 [details] [diff] [review]:
-----------------------------------------------------------------

Yep, this is a good start.  r+ usually means "patch ready to land", so I'll convert this to a feedback? request.

From irc: flattening should happen via JSString::ensureLinear/ensureFlat and will entail factoring out a JSString::ensureLinearSlow/ensureFlatSlow to dispatch to either rope or int-string.  After that, find some places where we can create JSIntString.  js_ValueToString is a good start; also js_IntToString (which can probably be wholly replaced by calls to JSIntString::new_; JSIntString::flatten should test for JSAtom::hasIntStatics, though) and IntToCString when base == 10.

With those places spewing JSIntStrings, then the fun part is to build a browser and see how many JSIntStrings get created (absolute and as a % of total strings) and what % of JSIntStrings get flattened, and where.

::: js/src/jstracer.cpp
@@ +12840,5 @@
>      idx_ins = w.ui2p(idx_ins);
>      LIns *lengthAndFlags_ins = w.ldpStringLengthAndFlags(str_ins);
> +    if (MaybeBranch mbr = w.jf(w.eqp(w.andp(lengthAndFlags_ins,
> +                                            w.nameImmw(JSString::ROPE_MASK)),
> +                                     w.nameImmw(JSString::ROPE_FLAGS))))

This code is inlining the linearity check (it's about to index into the char array), so you don't want to preserve testing for ropes, instead you want to test for linearity (still a single op as you pointed out) and have js_FlattenOnTrace handle ropes as well as int-strings.  Same comment for getCharAt and methodjit.

::: js/src/vm/String.h
@@ +228,5 @@
>       *   string       instance   subtype
>       *   type         encoding   predicate
>       *
>       *   String       -          true
> +     *   Rope         0001       xx01 *

Why *?

Comment 19

[Eddy Bruel [:ejpbruel]]

Hi all,

This bug had been shelved for a while due to me being away on holiday for a couple of weeks, so I thought I'd let you know that I've picked it up again.

So far, I've implemented JSIntString::flatten and refactored JSString::ensureLinear/ensureFlat, but I need a couple of pointers before I can proceed. Luke, could you please remind me why we needed a separate ensureLinearSlow and ensureFlatSlow again? (Since conditional statements are short-circuited they do not cause any overhead unless the check they perform is actually necessary, so the only reason I could come up with is that ensureLinear/ensureFlat could be used as a faster alternative when the result of one of the checks is known in advance. Or am I overlooking something?)

>This code is inlining the linearity check (it's about to index into the char array), so you don't want to preserve testing for >ropes, instead you want to test for linearity (still a single op as you pointed out) and have js_FlattenOnTrace handle ropes >as well as int-strings.  Same comment for getCharAt and methodjit.

I assume that what you mean here is that checking whether a string is a rope used to be equivalent to checking whether it is not linear, but with the introduction of int-strings, this is now no longer the case. Is this correct? Furthermore, does js_FlattenOnTrace have to do anything special to handle int-strings, or do I just have to refactor the check there as well?

>Why *?

That was a typo :-)

Comment 20

[Luke Wagner [:luke]]

(In reply to ejpbruel from comment #19)
> Luke, could you please remind me why we needed a separate
> ensureLinearSlow and ensureFlatSlow again?

The goal is not to add anything else to the always-inlined ensureLinear method.  An alternative would be to just add the int-string-flattening functionality to JSRope::flatten (which could then be renamed to JSString::linearize).

> I assume that what you mean here is that checking whether a string is a rope
> used to be equivalent to checking whether it is not linear, but with the
> introduction of int-strings, this is now no longer the case. Is this
> correct? Furthermore, does js_FlattenOnTrace have to do anything special to
> handle int-strings, or do I just have to refactor the check there as well?

Good news: the tracer is slated to be removed somewhere in the near future, so you could probably just ignore it for now (configure with --disable-tracer).

Comment 21

[Eddy Bruel [:ejpbruel]]

Attachment: Finished JSIntString

I've implement JSIntString::flatten such that it's almost anologous to what js_IntToString used to, except that instead of creating a JSShortString, it converts the existing JSIntString to a JSShortString in-place. Note that in order for this to work, JSIntStrings have to be allocated with js_NewGCShortString, and the isShort() test has to be extended with !isInt(), because both JSShortString and JSIntString are now allocated in the js::FINALIZE_SHORT_STRING arena.

There are several places in the code where it is assumed that if a string is not linear, it must be a rope, or if a string is not a rope, it must be linear. Since JSIntString introduces a second type of non-linear strings, this assumption is now no longer true.

I've solved the problem of keeping the inline size of ensureFlat and ensureLinear minimal by factoring out two functions ensureFlatSlow and ensureLinearSlow. These are called from ensureFlat and ensureLinear, but not inlined themselves. They should only be called when the string is known not to be already of the desired type.

The two places where I've added the creation of JSIntStrings so far is in js_ValueToString and IdToString. As far as I can tell, this makes js_IntToString now obsolete (there are still some references to it in the tracer, but as Luke pointed out, those can be ignored), so it can probably be removed later on. 

I've tested this patch with jstests.py in release mode and it causes no additional regressions. Please let me know if there are any improvements, changes, fixes, etc you want to see to this patch.

In the meantime, I will start building a browser and measure how many JSIntStrings actually get created. It might also be a good idea to measure if the use of JSIntString actually helps performance. Any suggestions how to go about that?

Comment 22

[Eddy Bruel [:ejpbruel]]

Attachment: Finished JSIntString (addendum)

Tom pointed out that I forgot to actually take advantage of the fact that we now have a JSIntString by adding a shortcut to StringToNumberType<T>. This patch addresses that oversight.

Comment 23

[Luke Wagner [:luke]]

Comment on attachment 570008 [details] [diff] [review]
Finished JSIntString (addendum)

Review of attachment 570008 [details] [diff] [review]:
-----------------------------------------------------------------

Great job!

Here are some other places to inject JSIntString fast-paths:
 - IsDefinitelyIndex
 - ValueFitsInInt32
 - num_parseInt
 - ValueToECMAInt32Slow/ValueToECMAUint32/ValueToUint16 (probably as the first case of the *Slow path)

I'll give an f+ now and switch it to r+ once we get some perf data.

(In reply to ejpbruel from comment #21)
> It might also be a good idea to measure
> if the use of JSIntString actually helps performance. Any suggestions how to
> go about that?

 1. compare SS 0.9.1, V8, and Kraken perf before/aft (--enable-optimize --disable-debug)
 2. instrument a browser to keep a set of global counters (printed in JS_DestroyRuntime):
   a. number of JSStrings finalized (++ at the top of JSString::finalize)
   b. number of JSShortStrings finalized (++ at the top of JSShortString::finalize)
   c. number of JSShortStrings that are JSIntStrings finalized (if (isInt() ++ in JSShortString::finalize)
    The ratio c/(a+b) should give us an indication of how often this optimization is winning.  I'd do a sanity check on a sample script (where you *know* the optimization should win), and then try this out on: SS, V8, Kraken and then various JS heavy sites you can think of (e.g., FB, GMail).

1 we must not regress and it would be fantastic if we improved. 2 can provide justification for the optimization if 1 doesn't.

I mostly have nits and one perf improvement:

::: js/src/vm/String-inl.h
@@ +94,5 @@
> +    d.u1.value = value;
> +}
> +
> +JS_ALWAYS_INLINE JSIntString *
> +JSIntString::new_(JSContext *cx, jsint value)

For new code, we never use jsint, so you can use 'int' everywhere.

@@ +109,5 @@
> +    }
> +    do { 
> +        ++length;
> +        ui /= 10;
> +    } while (ui > 0);

It would be good to avoid repeated int division as this can be quite slow.  I tried gcc -O3 using math.h's log10 hoping this would generate some fast inline asm, but no dice.  A simple if/else ladder should be good since log10(int) has a small range.  It would be good to hoist this into a reusable FloorLog10 function in jsutil.h.

@@ +112,5 @@
> +        ui /= 10;
> +    } while (ui > 0);
> +
> +    if (!validateLength(cx, length))
> +        return NULL;

You should be able to make a static assert that the max length is less than JSString::MAX_LENGTH.

::: js/src/vm/String.cpp
@@ +127,5 @@
> +JSString::ensureLinearSlow(JSContext *cx)
> +{
> +    return isRope()
> +         ? asRope().flatten(cx)
> +         : asInt().flatten(cx);

SM style is to align the ? with the i in isRope.

@@ +274,5 @@
>      }
>  }
>  
> +JSFlatString *
> +JSIntString::flatten(JSContext *maybecx)

Since this can't fail (wee!) you should remove this parameter (which, via SM style, indicates infallibility).

::: js/src/vm/String.h
@@ +181,5 @@
>          size_t                     lengthAndFlags;      /* JSString */
>          union {
>              const jschar           *chars;              /* JSLinearString */
>              JSString               *left;               /* JSRope */
> +            jsint                  value;               /* JSIntString */

s/jsint/int/

Comment 24

[Tom S [:evilpie]]

As I mentioned on IRC please make sure that this actually works with for ... in.
For example js_IteratorNext uses StaticStrings for int32. IternatorNext use JSID_TO_STRING if this is executed for int jsids.

Also some of the JägerMonkey stubs would probably like this. E.g. jsop_getelem (maybe even only inside for .. in). But you need to measure if this helps or even hurts performance overall.

Comment 25

[Emanuel Hoogeveen [:ehoogeveen]]

(In reply to Luke Wagner [:luke] from comment #23)
> @@ +109,5 @@
> > +    }
> > +    do { 
> > +        ++length;
> > +        ui /= 10;
> > +    } while (ui > 0);
> 
> It would be good to avoid repeated int division as this can be quite slow. 
> I tried gcc -O3 using math.h's log10 hoping this would generate some fast
> inline asm, but no dice.  A simple if/else ladder should be good since
> log10(int) has a small range.  It would be good to hoist this into a
> reusable FloorLog10 function in jsutil.h.
By an if/else ladder do you mean the method from
http://graphics.stanford.edu/~seander/bithacks.html#IntegerLog10Obvious ?
I guess you could change the ordering into a binary search for uniformly distributed output values (I have no idea how ints in strings tend to be distributed though, and the linear search has a much better best case).

Comment 26

[Luke Wagner [:luke]]

(In reply to Emanuel Hoogeveen from comment #25)
> By an if/else ladder do you mean the method from
> http://graphics.stanford.edu/~seander/bithacks.html#IntegerLog10Obvious ?

Looks about right, although I would have guessed small numbers are more common, favoring a reversed order.  This can be measured, though.

Comment 27

[Tom S [:evilpie]]

I just realized you should reuse intStaticTable for JSIntStrings, because we create these low numbers a lot.

Comment 28

[Eddy Bruel [:ejpbruel]]

After fixing a GC bug (because JSIntStrings are neither ropes or linear strings, they were not being marked), and injecting JSIntString in the additional places suggested by Luke, here are the initial results from running Kraken before and after applying the patch:

TEST                         COMPARISON            FROM                 TO               DETAILS

====================================================================================

** TOTAL **:                 1.008x as fast    3092.8ms +/- 0.6%   3068.9ms +/- 0.3%     significant

====================================================================================

  ai:                        -                  113.2ms +/- 1.2%    112.5ms +/- 0.9% 
    astar:                   -                  113.2ms +/- 1.2%    112.5ms +/- 0.9% 

  audio:                     -                  885.8ms +/- 1.0%    880.2ms +/- 0.9% 
    beat-detection:          -                  284.2ms +/- 1.4%    280.6ms +/- 1.3% 
    dft:                     ??                 249.6ms +/- 1.9%    252.8ms +/- 1.0%     might be *1.013x as slow*
    fft:                     1.017x as fast     174.8ms +/- 0.5%    171.9ms +/- 1.0%     significant
    oscillator:              -                  177.2ms +/- 1.8%    174.9ms +/- 2.1% 

  imaging:                   -                 1316.1ms +/- 0.7%   1307.2ms +/- 0.6% 
    gaussian-blur:           1.010x as fast     689.5ms +/- 0.4%    682.4ms +/- 0.3%     significant
    darkroom:                -                  394.2ms +/- 1.2%    392.4ms +/- 1.8% 
    desaturate:              -                  232.4ms +/- 2.9%    232.4ms +/- 1.5% 

  json:                      ??                 178.3ms +/- 0.8%    179.9ms +/- 0.9%     might be *1.009x as slow*
    parse-financial:         1.028x as fast      63.5ms +/- 1.0%     61.8ms +/- 0.9%     significant
    stringify-tinderbox:     *1.029x as slow*   114.8ms +/- 1.0%    118.1ms +/- 1.2%     significant

  stanford:                  1.017x as fast     599.4ms +/- 0.7%    589.1ms +/- 0.4%     significant
    crypto-aes:              -                  142.9ms +/- 0.8%    142.8ms +/- 0.7% 
    crypto-ccm:              1.019x as fast     113.5ms +/- 1.7%    111.4ms +/- 0.6%     significant
    crypto-pbkdf2:           1.021x as fast     237.4ms +/- 0.7%    232.6ms +/- 0.9%     significant
    crypto-sha256-iterative: 1.032x as fast     105.6ms +/- 1.1%    102.3ms +/- 0.8%     significant

There are a few benchmarks where we are regressing, but in the majority of cases, I'm seeing a small but noticable improvement. Note that these results were obtained without implementing the if/else ladder suggested above, so I suspect we can do even better. Stay tuned for additional results.

Comment 29

[Eddy Bruel [:ejpbruel]]

After having added the FloorLog10 function as suggested in comments #23 and #26, here are the numbers I'm seeing in Kraken:

Without JSIntString: 2983.9ms (http://tinyurl.com/68wf2vy)
With JSIntString, but without FloorLog10: 2951.1ms (http://tinyurl.com/5w8yr5h)
With JSIntString, and with FloorLog10: 2930.6ms (http://tinyurl.com/6eplsmb)

Of course, I've done multiple runs for each test, and there is some variance between them. During some runs, the version without JSIntString actually won, but on average, the version with both JSIntString and FloorLog10 performed better (which is what I expected). The above numbers are representative of the average results I got for each test.

Comment 30

[Eddy Bruel [:ejpbruel]]

Here are the results from SunSpider:

Without JSIntString: 171.8ms (http://tinyurl.com/6hotya8)
With JSIntString, and with FloorLog10: 167.2ms (http://tinyurl.com/6y7b3hz)

Comment 31

[Eddy Bruel [:ejpbruel]]

And here are the V8 benchmarks (note that bigger is better here):

Without JSIntString: 6712
With JSIntString, and with FloorLog10: 6935

Comment 32

[David Mandelin [:dmandelin]]

(In reply to ejpbruel from comment #31)
> And here are the V8 benchmarks (note that bigger is better here):
> 
> Without JSIntString: 6712
> With JSIntString, and with FloorLog10: 6935

Ooh, that would be great.

Comment 33

[Eddy Bruel [:ejpbruel]]

(In reply to David Mandelin from comment #32)
> (In reply to ejpbruel from comment #31)
> > And here are the V8 benchmarks (note that bigger is better here):
> > 
> > Without JSIntString: 6712
> > With JSIntString, and with FloorLog10: 6935
> 
> Ooh, that would be great.

Well, to be fair, I did some more runs of the V8 benchmark, and it seem to show more variance between runs than Kraken or SpiderMonkey. Based on the numbers I've seen so far, my guesstimate would be that we're looking at an improvement in the 0.1-1.0% range in the majority of cases, with the V8 numbers probably depicting a somewhat too pretty picture.

Comment 34

[Luke Wagner [:luke]]

Great results so far!  I'm interested to see the results of measurement (2) in comment 23.

Comment 35

[Eddy Bruel [:ejpbruel]]

Unfortunately, my patch causes Firefox to crash with gmail. I'm currently investigating what's the cause of this. In the meantime, here are the results of measurement (2) when running Kraken:

Strings finalized      : 2560599
Int strings finalized  : 21784
Short strings finalized: 2687517
Percentage int strings : 0.415082

Not very impressive, but then again I don't think Kraken is designed to be very heavy in it's use of integer strings.

Comment 36

[Eddy Bruel [:ejpbruel]]

Attachment: Current repo state; patch is causing Firefox to crash

I've been trying to figure out what I've changed that causes Firefox to crash when logging into GMail, unfortunately without success so far. A stack trace revealed that the crash happens at the same point every time, which is in the JIT (see below). I'm really not familiar enough with this part of SpiderMonkey to be able to figure this out on my own. I could use some guidance here.

I've attached a patch that reflects the state of my repository relative to when I started diverging from mozilla-central (revision cf5da681d577). I'd like to ask Luke to take a look at this patch to see if there is anything obvious that I'm doing wrong. If it helps, here's the stacktrace I got from gdb:

#0  0x00000001573d151f in ?? ()
#1  0x0000000102e7ba8c in js::mjit::EnterMethodJIT (cx=0x152d27200, fp=0x1195021b0, code=0x1573d0768, stackLimit=0x1198e0000, partial=true) at /Users/ejpbruel/Projects/mozilla-central-654190/js/src/methodjit/MethodJIT.cpp:985
#2  0x0000000102e7bcca in CheckStackAndEnterMethodJIT (cx=0x152d27200, fp=0x1195021b0, code=0x1573d0768, partial=true) at /Users/ejpbruel/Projects/mozilla-central-654190/js/src/methodjit/MethodJIT.cpp:1046
#3  0x0000000102e7bdf3 in js::mjit::JaegerShot (cx=0x152d27200, partial=true) at /Users/ejpbruel/Projects/mozilla-central-654190/js/src/methodjit/MethodJIT.cpp:1063
#4  0x0000000102c9e1e2 in js::Interpret (cx=0x152d27200, entryFrame=0x119501ee8, interpMode=js::JSINTERP_NORMAL) at /Users/ejpbruel/Projects/mozilla-central-654190/js/src/jsinterp.cpp:4036
#5  0x0000000102cad64f in js::RunScript (cx=0x152d27200, script=0x153b06040, fp=0x119501ee8) at /Users/ejpbruel/Projects/mozilla-central-654190/js/src/jsinterp.cpp:584
#6  0x0000000102cadd12 in js::InvokeKernel (cx=0x152d27200, args={<js::CallReceiver> = {usedRval_ = false, argv_ = 0x119501ee8}, argc_ = 0}, construct=js::NO_CONSTRUCT) at /Users/ejpbruel/Projects/mozilla-central-654190/js/src/jsinterp.cpp:647
#7  0x0000000102be9bc9 in js::Invoke (cx=0x152d27200, args=@0x7fff5fbe8230, construct=js::NO_CONSTRUCT) at jsinterp.h:148
#8  0x0000000102c44ca1 in js_fun_call (cx=0x152d27200, argc=0, vp=0x119501ec0) at /Users/ejpbruel/Projects/mozilla-central-654190/js/src/jsfun.cpp:1830
#9  0x0000000102cb0642 in js::CallJSNative (cx=0x152d27200, native=0x102c44b2d <js_fun_call(JSContext*, unsigned int, JS::Value*)>, args=@0x7fff5fbe83e0) at jscntxtinlines.h:297
#10 0x0000000102cadc3b in js::InvokeKernel (cx=0x152d27200, args={<js::CallReceiver> = {usedRval_ = false, argv_ = 0x119501ed0}, argc_ = 1}, construct=js::NO_CONSTRUCT) at /Users/ejpbruel/Projects/mozilla-central-654190/js/src/jsinterp.cpp:629
#11 0x0000000102c9dbd1 in js::Interpret (cx=0x152d27200, entryFrame=0x119501bc8, interpMode=js::JSINTERP_NORMAL) at /Users/ejpbruel/Projects/mozilla-central-654190/js/src/jsinterp.cpp:3997
#12 0x0000000102cad64f in js::RunScript (cx=0x152d27200, script=0x153b0d040, fp=0x119501bc8) at /Users/ejpbruel/Projects/mozilla-central-654190/js/src/jsinterp.cpp:584
#13 0x0000000102cadd12 in js::InvokeKernel (cx=0x152d27200, args={<js::CallReceiver> = {usedRval_ = false, argv_ = 0x119501bc8}, argc_ = 0}, construct=js::NO_CONSTRUCT) at /Users/ejpbruel/Projects/mozilla-central-654190/js/src/jsinterp.cpp:647
#14 0x0000000102be9bc9 in js::Invoke (cx=0x152d27200, args=@0x7fff5fbe95e0, construct=js::NO_CONSTRUCT) at jsinterp.h:148
#15 0x0000000102c44ca1 in js_fun_call (cx=0x152d27200, argc=0, vp=0x119501ba0) at /Users/ejpbruel/Projects/mozilla-central-654190/js/src/jsfun.cpp:1830
#16 0x0000000102cb0642 in js::CallJSNative (cx=0x152d27200, native=0x102c44b2d <js_fun_call(JSContext*, unsigned int, JS::Value*)>, args=@0x7fff5fbe9790) at jscntxtinlines.h:297
#17 0x0000000102cadc3b in js::InvokeKernel (cx=0x152d27200, args={<js::CallReceiver> = {usedRval_ = false, argv_ = 0x119501bb0}, argc_ = 1}, construct=js::NO_CONSTRUCT) at /Users/ejpbruel/Projects/mozilla-central-654190/js/src/jsinterp.cpp:629
#18 0x0000000102c9dbd1 in js::Interpret (cx=0x152d27200, entryFrame=0x1195018f8, interpMode=js::JSINTERP_NORMAL) at /Users/ejpbruel/Projects/mozilla-central-654190/js/src/jsinterp.cpp:3997
#19 0x0000000102cad64f in js::RunScript (cx=0x152d27200, script=0x153b0eac0, fp=0x1195018f8) at /Users/ejpbruel/Projects/mozilla-central-654190/js/src/jsinterp.cpp:584
#20 0x0000000102cadd12 in js::InvokeKernel (cx=0x152d27200, args={<js::CallReceiver> = {usedRval_ = false, argv_ = 0x1195018f0}, argc_ = 1}, construct=js::NO_CONSTRUCT) at /Users/ejpbruel/Projects/mozilla-central-654190/js/src/jsinterp.cpp:647
#21 0x0000000102be9bc9 in js::Invoke (cx=0x152d27200, args=@0x7fff5fbea990, construct=js::NO_CONSTRUCT) at jsinterp.h:148
#22 0x0000000102c44ca1 in js_fun_call (cx=0x152d27200, argc=1, vp=0x1195018c0) at /Users/ejpbruel/Projects/mozilla-central-654190/js/src/jsfun.cpp:1830
#23 0x0000000102cb0642 in js::CallJSNative (cx=0x152d27200, native=0x102c44b2d <js_fun_call(JSContext*, unsigned int, JS::Value*)>, args=@0x7fff5fbeab40) at jscntxtinlines.h:297
#24 0x0000000102cadc3b in js::InvokeKernel (cx=0x152d27200, args={<js::CallReceiver> = {usedRval_ = false, argv_ = 0x1195018d0}, argc_ = 2}, construct=js::NO_CONSTRUCT) at /Users/ejpbruel/Projects/mozilla-central-654190/js/src/jsinterp.cpp:629
#25 0x0000000102c9dbd1 in js::Interpret (cx=0x152d27200, entryFrame=0x119501690, interpMode=js::JSINTERP_NORMAL) at /Users/ejpbruel/Projects/mozilla-central-654190/js/src/jsinterp.cpp:3997
#26 0x0000000102cad64f in js::RunScript (cx=0x152d27200, script=0x153f11900, fp=0x119501690) at /Users/ejpbruel/Projects/mozilla-central-654190/js/src/jsinterp.cpp:584
#27 0x0000000102cadd12 in js::InvokeKernel (cx=0x152d27200, args={<js::CallReceiver> = {usedRval_ = false, argv_ = 0x119501688}, argc_ = 1}, construct=js::NO_CONSTRUCT) at /Users/ejpbruel/Projects/mozilla-central-654190/js/src/jsinterp.cpp:647
#28 0x0000000102be9bc9 in js::Invoke (cx=0x152d27200, args=@0x7fff5fbebd40, construct=js::NO_CONSTRUCT) at jsinterp.h:148
#29 0x0000000102c44ca1 in js_fun_call (cx=0x152d27200, argc=1, vp=0x119501658) at /Users/ejpbruel/Projects/mozilla-central-654190/js/src/jsfun.cpp:1830
#30 0x0000000102cb0642 in js::CallJSNative (cx=0x152d27200, native=0x102c44b2d <js_fun_call(JSContext*, unsigned int, JS::Value*)>, args=@0x7fff5fbebef0) at jscntxtinlines.h:297
#31 0x0000000102cadc3b in js::InvokeKernel (cx=0x152d27200, args={<js::CallReceiver> = {usedRval_ = false, argv_ = 0x119501668}, argc_ = 2}, construct=js::NO_CONSTRUCT) at /Users/ejpbruel/Projects/mozilla-central-654190/js/src/jsinterp.cpp:629
#32 0x0000000102c9dbd1 in js::Interpret (cx=0x152d27200, entryFrame=0x1195015e8, interpMode=js::JSINTERP_NORMAL) at /Users/ejpbruel/Projects/mozilla-central-654190/js/src/jsinterp.cpp:3997
#33 0x0000000102cad64f in js::RunScript (cx=0x152d27200, script=0x154536c80, fp=0x1195015e8) at /Users/ejpbruel/Projects/mozilla-central-654190/js/src/jsinterp.cpp:584
#34 0x0000000102cadd12 in js::InvokeKernel (cx=0x152d27200, args={<js::CallReceiver> = {usedRval_ = false, argv_ = 0x1195015e0}, argc_ = 1}, construct=js::NO_CONSTRUCT) at /Users/ejpbruel/Projects/mozilla-central-654190/js/src/jsinterp.cpp:647
#35 0x0000000102be9bc9 in js::Invoke (cx=0x152d27200, args=@0x7fff5fbed0f0, construct=js::NO_CONSTRUCT) at jsinterp.h:148
#36 0x0000000102c44ca1 in js_fun_call (cx=0x152d27200, argc=1, vp=0x1195015b0) at /Users/ejpbruel/Projects/mozilla-central-654190/js/src/jsfun.cpp:1830
#37 0x0000000102cb0642 in js::CallJSNative (cx=0x152d27200, native=0x102c44b2d <js_fun_call(JSContext*, unsigned int, JS::Value*)>, args=@0x7fff5fbed2a0) at jscntxtinlines.h:297
#38 0x0000000102cadc3b in js::InvokeKernel (cx=0x152d27200, args={<js::CallReceiver> = {usedRval_ = false, argv_ = 0x1195015c0}, argc_ = 2}, construct=js::NO_CONSTRUCT) at /Users/ejpbruel/Projects/mozilla-central-654190/js/src/jsinterp.cpp:629
#39 0x0000000102c9dbd1 in js::Interpret (cx=0x152d27200, entryFrame=0x119501410, interpMode=js::JSINTERP_NORMAL) at /Users/ejpbruel/Projects/mozilla-central-654190/js/src/jsinterp.cpp:3997
#40 0x0000000102f2f171 in UncachedInlineCall (f=@0x7fff5fbee580, initial=js::INITIAL_NONE, pret=0x7fff5fbee4c0, unjittable=0x7fff5fbee4c8, argc=5) at /Users/ejpbruel/Projects/mozilla-central-654190/js/src/methodjit/InvokeHelpers.cpp:392
#41 0x0000000102f2f2e3 in js::mjit::stubs::UncachedCallHelper (f=@0x7fff5fbee580, argc=5, lowered=false, ucr=0x7fff5fbee4b0) at /Users/ejpbruel/Projects/mozilla-central-654190/js/src/methodjit/InvokeHelpers.cpp:479
#42 0x0000000102f10f3a in CallCompiler::update (this=0x7fff5fbee550) at /Users/ejpbruel/Projects/mozilla-central-654190/js/src/methodjit/MonoIC.cpp:1090
#43 0x0000000102f05e43 in js::mjit::ic::Call (f=@0x7fff5fbee580, ic=0x156f492e0) at /Users/ejpbruel/Projects/mozilla-central-654190/js/src/methodjit/MonoIC.cpp:1150
#44 0x0000000156c6bf18 in ?? ()
#45 0x0000000102e7ba8c in js::mjit::EnterMethodJIT (cx=0x152d27200, fp=0x1195012b0, code=0x156ca42d0, stackLimit=0x1198e0000, partial=true) at /Users/ejpbruel/Projects/mozilla-central-654190/js/src/methodjit/MethodJIT.cpp:985
#46 0x0000000102e7bcca in CheckStackAndEnterMethodJIT (cx=0x152d27200, fp=0x1195012b0, code=0x156ca42d0, partial=true) at /Users/ejpbruel/Projects/mozilla-central-654190/js/src/methodjit/MethodJIT.cpp:1046
#47 0x0000000102e7bdf3 in js::mjit::JaegerShot (cx=0x152d27200, partial=true) at /Users/ejpbruel/Projects/mozilla-central-654190/js/src/methodjit/MethodJIT.cpp:1063
#48 0x0000000102c9e1e2 in js::Interpret (cx=0x152d27200, entryFrame=0x1195010e8, interpMode=js::JSINTERP_NORMAL) at /Users/ejpbruel/Projects/mozilla-central-654190/js/src/jsinterp.cpp:4036
#49 0x0000000102cad64f in js::RunScript (cx=0x152d27200, script=0x153b6b820, fp=0x1195010e8) at /Users/ejpbruel/Projects/mozilla-central-654190/js/src/jsinterp.cpp:584
#50 0x0000000102cadd12 in js::InvokeKernel (cx=0x152d27200, args={<js::CallReceiver> = {usedRval_ = false, argv_ = 0x1195010e0}, argc_ = 1}, construct=js::NO_CONSTRUCT) at /Users/ejpbruel/Projects/mozilla-central-654190/js/src/jsinterp.cpp:647
#51 0x0000000102be9bc9 in js::Invoke (cx=0x152d27200, args=@0x7fff5fbef980, construct=js::NO_CONSTRUCT) at jsinterp.h:148
#52 0x0000000102c44ca1 in js_fun_call (cx=0x152d27200, argc=1, vp=0x1195010b0) at /Users/ejpbruel/Projects/mozilla-central-654190/js/src/jsfun.cpp:1830
#53 0x0000000102cb0642 in js::CallJSNative (cx=0x152d27200, native=0x102c44b2d <js_fun_call(JSContext*, unsigned int, JS::Value*)>, args=@0x7fff5fbefb30) at jscntxtinlines.h:297
#54 0x0000000102cadc3b in js::InvokeKernel (cx=0x152d27200, args={<js::CallReceiver> = {usedRval_ = false, argv_ = 0x1195010c0}, argc_ = 2}, construct=js::NO_CONSTRUCT) at /Users/ejpbruel/Projects/mozilla-central-654190/js/src/jsinterp.cpp:629
#55 0x0000000102c9dbd1 in js::Interpret (cx=0x152d27200, entryFrame=0x119500f10, interpMode=js::JSINTERP_NORMAL) at /Users/ejpbruel/Projects/mozilla-central-654190/js/src/jsinterp.cpp:3997
#56 0x0000000102f2f171 in UncachedInlineCall (f=@0x7fff5fbf0e10, initial=js::INITIAL_NONE, pret=0x7fff5fbf0d50, unjittable=0x7fff5fbf0d58, argc=5) at /Users/ejpbruel/Projects/mozilla-central-654190/js/src/methodjit/InvokeHelpers.cpp:392
#57 0x0000000102f2f2e3 in js::mjit::stubs::UncachedCallHelper (f=@0x7fff5fbf0e10, argc=5, lowered=false, ucr=0x7fff5fbf0d40) at /Users/ejpbruel/Projects/mozilla-central-654190/js/src/methodjit/InvokeHelpers.cpp:479
#58 0x0000000102f10f3a in CallCompiler::update (this=0x7fff5fbf0de0) at /Users/ejpbruel/Projects/mozilla-central-654190/js/src/methodjit/MonoIC.cpp:1090
#59 0x0000000102f05e43 in js::mjit::ic::Call (f=@0x7fff5fbf0e10, ic=0x156f492e0) at /Users/ejpbruel/Projects/mozilla-central-654190/js/src/methodjit/MonoIC.cpp:1150
#60 0x0000000156c6bf18 in ?? ()
#61 0x0000000102e7ba8c in js::mjit::EnterMethodJIT (cx=0x152d27200, fp=0x119500db0, code=0x156ca42d0, stackLimit=0x1198e0000, partial=true) at /Users/ejpbruel/Projects/mozilla-central-654190/js/src/methodjit/MethodJIT.cpp:985
#62 0x0000000102e7bcca in CheckStackAndEnterMethodJIT (cx=0x152d27200, fp=0x119500db0, code=0x156ca42d0, partial=true) at /Users/ejpbruel/Projects/mozilla-central-654190/js/src/methodjit/MethodJIT.cpp:1046
#63 0x0000000102e7bdf3 in js::mjit::JaegerShot (cx=0x152d27200, partial=true) at /Users/ejpbruel/Projects/mozilla-central-654190/js/src/methodjit/MethodJIT.cpp:1063
#64 0x0000000102c9e1e2 in js::Interpret (cx=0x152d27200, entryFrame=0x119500af0, interpMode=js::JSINTERP_NORMAL) at /Users/ejpbruel/Projects/mozilla-central-654190/js/src/jsinterp.cpp:4036
#65 0x0000000102cad64f in js::RunScript (cx=0x152d27200, script=0x152c0b4a0, fp=0x119500af0) at /Users/ejpbruel/Projects/mozilla-central-654190/js/src/jsinterp.cpp:584
#66 0x0000000102cadd12 in js::InvokeKernel (cx=0x152d27200, args={<js::CallReceiver> = {usedRval_ = false, argv_ = 0x119500ae8}, argc_ = 1}, construct=js::NO_CONSTRUCT) at /Users/ejpbruel/Projects/mozilla-central-654190/js/src/jsinterp.cpp:647
#67 0x0000000102be9bc9 in js::Invoke (cx=0x152d27200, args=@0x7fff5fbf2210, construct=js::NO_CONSTRUCT) at jsinterp.h:148
#68 0x0000000102c44ca1 in js_fun_call (cx=0x152d27200, argc=1, vp=0x119500ab8) at /Users/ejpbruel/Projects/mozilla-central-654190/js/src/jsfun.cpp:1830
#69 0x0000000102cb0642 in js::CallJSNative (cx=0x152d27200, native=0x102c44b2d <js_fun_call(JSContext*, unsigned int, JS::Value*)>, args=@0x7fff5fbf23c0) at jscntxtinlines.h:297
#70 0x0000000102cadc3b in js::InvokeKernel (cx=0x152d27200, args={<js::CallReceiver> = {usedRval_ = false, argv_ = 0x119500ac8}, argc_ = 2}, construct=js::NO_CONSTRUCT) at /Users/ejpbruel/Projects/mozilla-central-654190/js/src/jsinterp.cpp:629
#71 0x0000000102c9dbd1 in js::Interpret (cx=0x152d27200, entryFrame=0x1195009a0, interpMode=js::JSINTERP_NORMAL) at /Users/ejpbruel/Projects/mozilla-central-654190/js/src/jsinterp.cpp:3997
#72 0x0000000102cad64f in js::RunScript (cx=0x152d27200, script=0x153b01d60, fp=0x1195009a0) at /Users/ejpbruel/Projects/mozilla-central-654190/js/src/jsinterp.cpp:584
#73 0x0000000102cadd12 in js::InvokeKernel (cx=0x152d27200, args={<js::CallReceiver> = {usedRval_ = false, argv_ = 0x119500990}, argc_ = 2}, construct=js::NO_CONSTRUCT) at /Users/ejpbruel/Projects/mozilla-central-654190/js/src/jsinterp.cpp:647
#74 0x0000000102be9bc9 in js::Invoke (cx=0x152d27200, args=@0x7fff5fbf35c0, construct=js::NO_CONSTRUCT) at jsinterp.h:148
#75 0x0000000102c44ca1 in js_fun_call (cx=0x152d27200, argc=2, vp=0x119500958) at /Users/ejpbruel/Projects/mozilla-central-654190/js/src/jsfun.cpp:1830
#76 0x0000000102cb0642 in js::CallJSNative (cx=0x152d27200, native=0x102c44b2d <js_fun_call(JSContext*, unsigned int, JS::Value*)>, args=@0x7fff5fbf3770) at jscntxtinlines.h:297
#77 0x0000000102cadc3b in js::InvokeKernel (cx=0x152d27200, args={<js::CallReceiver> = {usedRval_ = false, argv_ = 0x119500968}, argc_ = 3}, construct=js::NO_CONSTRUCT) at /Users/ejpbruel/Projects/mozilla-central-654190/js/src/jsinterp.cpp:629
#78 0x0000000102c9dbd1 in js::Interpret (cx=0x152d27200, entryFrame=0x1195007b8, interpMode=js::JSINTERP_NORMAL) at /Users/ejpbruel/Projects/mozilla-central-654190/js/src/jsinterp.cpp:3997
#79 0x0000000102cad64f in js::RunScript (cx=0x152d27200, script=0x153b019e0, fp=0x1195007b8) at /Users/ejpbruel/Projects/mozilla-central-654190/js/src/jsinterp.cpp:584
#80 0x0000000102cadd12 in js::InvokeKernel (cx=0x152d27200, args={<js::CallReceiver> = {usedRval_ = false, argv_ = 0x1195007a8}, argc_ = 2}, construct=js::NO_CONSTRUCT) at /Users/ejpbruel/Projects/mozilla-central-654190/js/src/jsinterp.cpp:647
#81 0x0000000102be9bc9 in js::Invoke (cx=0x152d27200, args=@0x7fff5fbf4950, construct=js::NO_CONSTRUCT) at jsinterp.h:148
#82 0x0000000102c44ad3 in js::CallOrConstructBoundFunction (cx=0x152d27200, argc=2, vp=0x119500778) at /Users/ejpbruel/Projects/mozilla-central-654190/js/src/jsfun.cpp:2005
#83 0x0000000102cb0642 in js::CallJSNative (cx=0x152d27200, native=0x102c448ef <js::CallOrConstructBoundFunction(JSContext*, unsigned int, JS::Value*)>, args=@0x7fff5fbf4af0) at jscntxtinlines.h:297
#84 0x0000000102cadc3b in js::InvokeKernel (cx=0x152d27200, args={<js::CallReceiver> = {usedRval_ = false, argv_ = 0x119500788}, argc_ = 2}, construct=js::NO_CONSTRUCT) at /Users/ejpbruel/Projects/mozilla-central-654190/js/src/jsinterp.cpp:629
#85 0x0000000102c9dbd1 in js::Interpret (cx=0x152d27200, entryFrame=0x119500700, interpMode=js::JSINTERP_NORMAL) at /Users/ejpbruel/Projects/mozilla-central-654190/js/src/jsinterp.cpp:3997
#86 0x0000000102cad64f in js::RunScript (cx=0x152d27200, script=0x153b6be40, fp=0x119500700) at /Users/ejpbruel/Projects/mozilla-central-654190/js/src/jsinterp.cpp:584
#87 0x0000000102cadd12 in js::InvokeKernel (cx=0x152d27200, args={<js::CallReceiver> = {usedRval_ = false, argv_ = 0x1195006e8}, argc_ = 1}, construct=js::NO_CONSTRUCT) at /Users/ejpbruel/Projects/mozilla-central-654190/js/src/jsinterp.cpp:647
#88 0x0000000102be9bc9 in js::Invoke (cx=0x152d27200, args=@0x7fff5fbf5cf0, construct=js::NO_CONSTRUCT) at jsinterp.h:148
#89 0x0000000102c44ca1 in js_fun_call (cx=0x152d27200, argc=1, vp=0x1195006b8) at /Users/ejpbruel/Projects/mozilla-central-654190/js/src/jsfun.cpp:1830
#90 0x0000000102cb0642 in js::CallJSNative (cx=0x152d27200, native=0x102c44b2d <js_fun_call(JSContext*, unsigned int, JS::Value*)>, args=@0x7fff5fbf5ea0) at jscntxtinlines.h:297
#91 0x0000000102cadc3b in js::InvokeKernel (cx=0x152d27200, args={<js::CallReceiver> = {usedRval_ = false, argv_ = 0x1195006c8}, argc_ = 2}, construct=js::NO_CONSTRUCT) at /Users/ejpbruel/Projects/mozilla-central-654190/js/src/jsinterp.cpp:629
#92 0x0000000102c9dbd1 in js::Interpret (cx=0x152d27200, entryFrame=0x119500518, interpMode=js::JSINTERP_NORMAL) at /Users/ejpbruel/Projects/mozilla-central-654190/js/src/jsinterp.cpp:3997
#93 0x0000000102f2f171 in UncachedInlineCall (f=@0x7fff5fbf7180, initial=js::INITIAL_NONE, pret=0x7fff5fbf70c0, unjittable=0x7fff5fbf70c8, argc=5) at /Users/ejpbruel/Projects/mozilla-central-654190/js/src/methodjit/InvokeHelpers.cpp:392
#94 0x0000000102f2f2e3 in js::mjit::stubs::UncachedCallHelper (f=@0x7fff5fbf7180, argc=5, lowered=false, ucr=0x7fff5fbf70b0) at /Users/ejpbruel/Projects/mozilla-central-654190/js/src/methodjit/InvokeHelpers.cpp:479
#95 0x0000000102f10f3a in CallCompiler::update (this=0x7fff5fbf7150) at /Users/ejpbruel/Projects/mozilla-central-654190/js/src/methodjit/MonoIC.cpp:1090
#96 0x0000000102f05e43 in js::mjit::ic::Call (f=@0x7fff5fbf7180, ic=0x156f49278) at /Users/ejpbruel/Projects/mozilla-central-654190/js/src/methodjit/MonoIC.cpp:1150
#97 0x0000000156c6a88e in ?? ()
#98 0x0000000102e7ba8c in js::mjit::EnterMethodJIT (cx=0x152d27200, fp=0x119500440, code=0x156c62e60, stackLimit=0x1198e0000, partial=true) at /Users/ejpbruel/Projects/mozilla-central-654190/js/src/methodjit/MethodJIT.cpp:985
#99 0x0000000102e7bcca in CheckStackAndEnterMethodJIT (cx=0x152d27200, fp=0x119500440, code=0x156c62e60, partial=true) at /Users/ejpbruel/Projects/mozilla-central-654190/js/src/methodjit/MethodJIT.cpp:1046
#100 0x0000000102e7bdf3 in js::mjit::JaegerShot (cx=0x152d27200, partial=true) at /Users/ejpbruel/Projects/mozilla-central-654190/js/src/methodjit/MethodJIT.cpp:1063
#101 0x0000000102c9e1e2 in js::Interpret (cx=0x152d27200, entryFrame=0x119500338, interpMode=js::JSINTERP_NORMAL) at /Users/ejpbruel/Projects/mozilla-central-654190/js/src/jsinterp.cpp:4036
#102 0x0000000102cad64f in js::RunScript (cx=0x152d27200, script=0x1343734a0, fp=0x119500338) at /Users/ejpbruel/Projects/mozilla-central-654190/js/src/jsinterp.cpp:584
#103 0x0000000102cadd12 in js::InvokeKernel (cx=0x152d27200, args={<js::CallReceiver> = {usedRval_ = false, argv_ = 0x119500338}, argc_ = 0}, construct=js::NO_CONSTRUCT) at /Users/ejpbruel/Projects/mozilla-central-654190/js/src/jsinterp.cpp:647
#104 0x0000000102be9bc9 in js::Invoke (cx=0x152d27200, args=@0x7fff5fbf8580, construct=js::NO_CONSTRUCT) at jsinterp.h:148
#105 0x0000000102c44ca1 in js_fun_call (cx=0x152d27200, argc=0, vp=0x119500310) at /Users/ejpbruel/Projects/mozilla-central-654190/js/src/jsfun.cpp:1830
#106 0x0000000102cb0642 in js::CallJSNative (cx=0x152d27200, native=0x102c44b2d <js_fun_call(JSContext*, unsigned int, JS::Value*)>, args=@0x7fff5fbf8730) at jscntxtinlines.h:297
#107 0x0000000102cadc3b in js::InvokeKernel (cx=0x152d27200, args={<js::CallReceiver> = {usedRval_ = false, argv_ = 0x119500320}, argc_ = 1}, construct=js::NO_CONSTRUCT) at /Users/ejpbruel/Projects/mozilla-central-654190/js/src/jsinterp.cpp:629
#108 0x0000000102c9dbd1 in js::Interpret (cx=0x152d27200, entryFrame=0x119500228, interpMode=js::JSINTERP_NORMAL) at /Users/ejpbruel/Projects/mozilla-central-654190/js/src/jsinterp.cpp:3997
#109 0x0000000102cad64f in js::RunScript (cx=0x152d27200, script=0x1343733c0, fp=0x119500228) at /Users/ejpbruel/Projects/mozilla-central-654190/js/src/jsinterp.cpp:584
#110 0x0000000102cadd12 in js::InvokeKernel (cx=0x152d27200, args={<js::CallReceiver> = {usedRval_ = false, argv_ = 0x119500210}, argc_ = 1}, construct=js::NO_CONSTRUCT) at /Users/ejpbruel/Projects/mozilla-central-654190/js/src/jsinterp.cpp:647
#111 0x0000000102be9bc9 in js::Invoke (cx=0x152d27200, args=@0x7fff5fbf9910, construct=js::NO_CONSTRUCT) at jsinterp.h:148
#112 0x0000000102c44ad3 in js::CallOrConstructBoundFunction (cx=0x152d27200, argc=1, vp=0x1195001e8) at /Users/ejpbruel/Projects/mozilla-central-654190/js/src/jsfun.cpp:2005
#113 0x0000000102cb0642 in js::CallJSNative (cx=0x152d27200, native=0x102c448ef <js::CallOrConstructBoundFunction(JSContext*, unsigned int, JS::Value*)>, args=@0x7fff5fbf9ab0) at jscntxtinlines.h:297
#114 0x0000000102cadc3b in js::InvokeKernel (cx=0x152d27200, args={<js::CallReceiver> = {usedRval_ = false, argv_ = 0x1195001f8}, argc_ = 1}, construct=js::NO_CONSTRUCT) at /Users/ejpbruel/Projects/mozilla-central-654190/js/src/jsinterp.cpp:629
#115 0x0000000102be9bc9 in js::Invoke (cx=0x152d27200, args=@0x7fff5fbf9b60, construct=js::NO_CONSTRUCT) at jsinterp.h:148
#116 0x0000000102c44f4b in js_fun_apply (cx=0x152d27200, argc=2, vp=0x1195001c8) at /Users/ejpbruel/Projects/mozilla-central-654190/js/src/jsfun.cpp:1886
#117 0x0000000102cb0642 in js::CallJSNative (cx=0x152d27200, native=0x102c44cdd <js_fun_apply(JSContext*, unsigned int, JS::Value*)>, args=@0x7fff5fbf9d00) at jscntxtinlines.h:297
#118 0x0000000102cadc3b in js::InvokeKernel (cx=0x152d27200, args={<js::CallReceiver> = {usedRval_ = false, argv_ = 0x1195001d8}, argc_ = 2}, construct=js::NO_CONSTRUCT) at /Users/ejpbruel/Projects/mozilla-central-654190/js/src/jsinterp.cpp:629
#119 0x0000000102c9dbd1 in js::Interpret (cx=0x152d27200, entryFrame=0x119500160, interpMode=js::JSINTERP_NORMAL) at /Users/ejpbruel/Projects/mozilla-central-654190/js/src/jsinterp.cpp:3997
#120 0x0000000102cad64f in js::RunScript (cx=0x152d27200, script=0x119a61660, fp=0x119500160) at /Users/ejpbruel/Projects/mozilla-central-654190/js/src/jsinterp.cpp:584
#121 0x0000000102cadd12 in js::InvokeKernel (cx=0x152d27200, args={<js::CallReceiver> = {usedRval_ = false, argv_ = 0x119500148}, argc_ = 1}, construct=js::NO_CONSTRUCT) at /Users/ejpbruel/Projects/mozilla-central-654190/js/src/jsinterp.cpp:647
#122 0x0000000102be9bc9 in js::Invoke (cx=0x152d27200, args=@0x7fff5fbfaf00, construct=js::NO_CONSTRUCT) at jsinterp.h:148
#123 0x0000000102c44f4b in js_fun_apply (cx=0x152d27200, argc=2, vp=0x119500118) at /Users/ejpbruel/Projects/mozilla-central-654190/js/src/jsfun.cpp:1886
#124 0x0000000102cb0642 in js::CallJSNative (cx=0x152d27200, native=0x102c44cdd <js_fun_apply(JSContext*, unsigned int, JS::Value*)>, args=@0x7fff5fbfb0a0) at jscntxtinlines.h:297
#125 0x0000000102cadc3b in js::InvokeKernel (cx=0x152d27200, args={<js::CallReceiver> = {usedRval_ = false, argv_ = 0x119500128}, argc_ = 2}, construct=js::NO_CONSTRUCT) at /Users/ejpbruel/Projects/mozilla-central-654190/js/src/jsinterp.cpp:629
#126 0x0000000102c9dbd1 in js::Interpret (cx=0x152d27200, entryFrame=0x1195000b0, interpMode=js::JSINTERP_NORMAL) at /Users/ejpbruel/Projects/mozilla-central-654190/js/src/jsinterp.cpp:3997
#127 0x0000000102cad64f in js::RunScript (cx=0x152d27200, script=0x134301660, fp=0x1195000b0) at /Users/ejpbruel/Projects/mozilla-central-654190/js/src/jsinterp.cpp:584
#128 0x0000000102cadd12 in js::InvokeKernel (cx=0x152d27200, args={<js::CallReceiver> = {usedRval_ = false, argv_ = 0x119500098}, argc_ = 1}, construct=js::NO_CONSTRUCT) at /Users/ejpbruel/Projects/mozilla-central-654190/js/src/jsinterp.cpp:647
#129 0x0000000102be9bc9 in js::Invoke (cx=0x152d27200, args=@0x7fff5fbfc2a0, construct=js::NO_CONSTRUCT) at jsinterp.h:148
#130 0x0000000102cae0b2 in js::Invoke (cx=0x152d27200, thisv=@0x7fff5fbfc350, fval=@0x7fff5fbfc318, argc=1, argv=0x1342262c8, rval=0x7fff5fbfc550) at /Users/ejpbruel/Projects/mozilla-central-654190/js/src/jsinterp.cpp:679
#131 0x0000000102bbb465 in JS_CallFunctionValue (cx=0x152d27200, obj=0x13432d128, fval={data = {asBits = 18445477442056742496, debugView = {payload47 = 5742388832, tag = JSVAL_TAG_OBJECT}, s = {payload = {i32 = 1447421536, u32 = 1447421536, why = 1447421536}}, asDouble = -nan(0xb80015645e660), asPtr = 0xfffb80015645e660, asWord = 18445477442056742496}}, argc=1, argv=0x1342262c8, rval=0x7fff5fbfc550) at /Users/ejpbruel/Projects/mozilla-central-654190/js/src/jsapi.cpp:5134
#132 0x0000000101a40283 in nsJSContext::CallEventHandler (this=0x1352fee10, aTarget=0x119d4b800, aScope=0x13432d128, aHandler=0x15645e660, aargv=0x156d6dd48, arv=0x7fff5fbfc7f0) at /Users/ejpbruel/Projects/mozilla-central-654190/dom/base/nsJSEnvironment.cpp:1946
#133 0x0000000101a7d644 in nsGlobalWindow::RunTimeout (this=0x119d4b800, aTimeout=0x156b69e40) at /Users/ejpbruel/Projects/mozilla-central-654190/dom/base/nsGlobalWindow.cpp:9279
#134 0x0000000101a7dc6d in nsGlobalWindow::TimerCallback (aTimer=0x156ebe080, aClosure=0x156b69e40) at /Users/ejpbruel/Projects/mozilla-central-654190/dom/base/nsGlobalWindow.cpp:9717
#135 0x00000001027a004f in nsTimerImpl::Fire (this=0x156ebe080) at /Users/ejpbruel/Projects/mozilla-central-654190/xpcom/threads/nsTimerImpl.cpp:424
#136 0x00000001027a030e in nsTimerEvent::Run (this=0x156e13730) at /Users/ejpbruel/Projects/mozilla-central-654190/xpcom/threads/nsTimerImpl.cpp:520
#137 0x00000001027987c6 in nsThread::ProcessNextEvent (this=0x100314b80, mayWait=false, result=0x7fff5fbfcbaf) at /Users/ejpbruel/Projects/mozilla-central-654190/xpcom/threads/nsThread.cpp:631
#138 0x000000010272580e in NS_ProcessPendingEvents_P (thread=0x100314b80, timeout=20) at /Users/ejpbruel/Projects/mozilla-central-654190/xpcom/build/nsThreadUtils.cpp:195
#139 0x00000001024f6ff5 in nsBaseAppShell::NativeEventCallback (this=0x100307a20) at /Users/ejpbruel/Projects/mozilla-central-654190/widget/src/xpwidgets/nsBaseAppShell.cpp:130
#140 0x00000001024a2ed4 in nsAppShell::ProcessGeckoEvents (aInfo=0x100307a20) at /Users/ejpbruel/Projects/mozilla-central-654190/widget/src/cocoa/nsAppShell.mm:424
#141 0x00007fff85038401 in __CFRunLoopDoSources0 ()
#142 0x00007fff850365f9 in __CFRunLoopRun ()
#143 0x00007fff85035dbf in CFRunLoopRunSpecific ()
#144 0x00007fff8610574e in RunCurrentEventLoopInMode ()
#145 0x00007fff86105553 in ReceiveNextEventCommon ()
#146 0x00007fff8610540c in BlockUntilNextEventMatchingListInMode ()
#147 0x00007fff800bce02 in _DPSNextEvent ()
#148 0x00007fff800bc751 in -[NSApplication nextEventMatchingMask:untilDate:inMode:dequeue:] ()
#149 0x00007fff80082427 in -[NSApplication run] ()
#150 0x00000001024a27f8 in nsAppShell::Run (this=0x100307a20) at /Users/ejpbruel/Projects/mozilla-central-654190/widget/src/cocoa/nsAppShell.mm:771
#151 0x00000001021eef17 in nsAppStartup::Run (this=0x1176af470) at /Users/ejpbruel/Projects/mozilla-central-654190/toolkit/components/startup/nsAppStartup.cpp:228
#152 0x000000010100e9fb in XRE_main (argc=1, argv=0x7fff5fbff1a0, aAppData=0x1003059b0) at /Users/ejpbruel/Projects/mozilla-central-654190/toolkit/xre/nsAppRunner.cpp:3585
#153 0x0000000100001b4d in do_main (exePath=0x7fff5fbfed70 "/Users/ejpbruel/Projects/mozilla-central-654190/dist/NightlyDebug.app/Contents/MacOS/libxpcom.dylib", argc=1, argv=0x7fff5fbff1a0) at /Users/ejpbruel/Projects/mozilla-central-654190/browser/app/nsBrowserApp.cpp:198
#154 0x0000000100001db4 in main (argc=1, argv=0x7fff5fbff1a0) at /Users/ejpbruel/Projects/mozilla-central-654190/browser/app/nsBrowserApp.cpp:281

Comment 37

[Luke Wagner [:luke]]

Comment on attachment 573832 [details] [diff] [review]
Current repo state; patch is causing Firefox to crash

From IRC: PushMarkStack needs to mark 'str' if 'str' is an IntString.

Comment 38

[Eddy Bruel [:ejpbruel]]

The fix suggested by Luke resolved the crash. Here are the result of measurement (2) after visiting some string heavy sites (Gmail/Twitter/several forums) for a couple of minutes:

Strings finalized      : 203351
Int strings finalized  : 11400
Short strings finalized: 125956
Percentage int strings : 3.461815

Comment 39

[Eddy Bruel [:ejpbruel]]

Awaiting results from try

Comment 40

[:Gijs (he/him)]

What's the status of this bug? Eddy?

The Dep that you added doesn't really look related to me... (although I'm obviously not an expert)

Comment 41

[Eddy Bruel [:ejpbruel]]

(In reply to Gijs Kruitbosch from comment #40)
> What's the status of this bug? Eddy?
> 
> The Dep that you added doesn't really look related to me... (although I'm
> obviously not an expert)

You are right, its not really related. Iirc, that dep was an intermittent segfault that was apparently turned permanent by my patch. Its unlikely my patch is directly responsible for that, but even so, I've been unable to land because of it. What's more, I didn't have any hope of resolving that dep by myself, so this patch has been stuck in limbo ever since.

If you feel like giving it another try, that would absolutely rock, so go right ahead! The patch is otherwise complete, and if you're lucky, the segfault might by now be intermittent again (my understanding was that the segfault was due to some threading issues, so it might be that the patch changed some timing parameters that made it much more likely for some race condition to occur)

Hope that helps. If you have any other questions, don't hesitate to ask!

Hope that helps.

Comment 42

[Tom S [:evilpie]]

Attachment: wip

Rebased and updated a little bit. Can't start the browser with it, but seems to pass jit-tests. Meh :/ I hate debugging the browser. This also has some code for measurements in it.

Comment 43

[Tom S [:evilpie]]

So I think, I need to come up with a different solution. After browsing around for a bit about 95% of all integer strings are flattened. V8 can just attach an index (integer) to every string which represents a number, so they have have the best of both fast integer conversion, but also fast string comparison, chaining etc. Most of the time we seem to flatten them in IdToString and js_SuppressDeletedProperty, the later is kind of weird, but I only went to gmail.

Comment 44

[Tom S [:evilpie]]

Attachment: Stack trace of flattening for some browsing

"Short" log of browser start and visiting google and slashdot.

A few items that stand out:
- NativeIterator::allocateIterator
We currently flatten every string we put into out NativeIterator, to really speed up the code mentioned in comment #0 we would need to stop doing this. Because at the moment for (var i in [1, 2, 3, 4]) {} always flattens the index , which would be representable as IntegerString, right away. I am not sure how easy that would be to solve, because at the moment IntStrings need more space than normal JSStrings, but we allocate the iterator in one big chunk like malloc(JSString * items).

-js_SuppressDeletedProperty
Similar to to the above to find the right entry in the props array we always flatten the string/property we are looking for

-IdToString
Probably need to look at every caller, but js_SuppressDeletedProperty seems to be big.

-ArgToRootedString
Probably not solvable unless we implement some hack for every function which uses it.

-Parser::memberExpr
http://mxr.mozilla.org/mozilla-central/source/js/src/frontend/Parser.cpp#5810
To decide whether to use by value or by property lookup we first convert every number to a string and then test if it's representable as an index!

Comment 45

[Tom S [:evilpie]]

Attachment: wip new approach

So after my recent findings, I changed my approach somewhat. 

- Int strings are still allocated like short strings.
- Int strings are now always flat.
- If we can we, will fit the characters inline, if not we allocate.

I made some changes that are most likely GC unsafe, but I wanted to get something to test on.

For this benchmark https://gist.github.com/3132685 we are at 0.29s with the patch and 0.36s without (trunk). So that is quite useful already, but I think this can be improved somewhat by extending the dense array IC to handle int strings, which should be really easy to do.

Comment 46

[Tom S [:evilpie]]

Attachment: proof of concept ionmonkey inline cache

I wrote this small patch, to test how the times would look like if we would inline cache access on dense array elements with integer strings. I first wanted to do that in Jäger, but man Ion IC code is like hundred times easier.

Before I got around 0.185 seconds in Ion, and with the patch around 0.099 seconds.

Comment 47

[Tom S [:evilpie]]

Attachment: refreshed version

This actually already looks quite good already.

Comment 48

[Tom S [:evilpie]]

https://tbpl.mozilla.org/?tree=Try&rev=8e48dd5ce5b6

Comment 49

[Tom S [:evilpie]]

Attachment: v1

I think this starting to look really nice. This passes try and doesn't fail some light browsing around. But I still want to make some more extensive measurements before we land this. There are probably more places where handling Int32Strings in JIT code would help. Doing this in the IonMonkey GetElem IC was a win already, but maybe for cases like ToInt32, Truncate it would make sense, too.

Comment 50

[Luke Wagner [:luke]]

Comment on attachment 653100 [details] [diff] [review]
v1

Review of attachment 653100 [details] [diff] [review]:
-----------------------------------------------------------------

I gave a first high-level look and I have some high-level questions:

First regarding measurements: are there any speedups on the benchmarks or measurements for how often this optimization hits for big webapps?  Also, could you give a few micro-benchmarks that represent the type of code this optimization speeds up and their timings?

Next, two ideas:
1. could we have the invariant that any integer id is a JSInt32Atom?  That would make IdToString fast in the non-special case and I think work well with NativeIterator.
2. hashing int32_t --> String should be pretty fast.  Do you think it would make sense to try to hash/lookup *every* integer that we converted into a string?  If we put them in the atoms compartment, then we potentially increase atom table sweep time, but we could have a compartment-local int32 -> string hash table that gets cleared on every GC.  Presumably, if the table time was significant, we could use a fixed-size table where the hash was int-value % table_size and clobber on collisions.

::: js/src/jsatominlines.h
@@ +127,5 @@
> +    RootedString str(cx);
> +    if (JS_LIKELY(JSID_IS_INT(id))) {
> +        str = Int32ToString(cx, JSID_TO_INT(id));
> +    } else {
> +        str = ToStringSlow(cx, IdToValue(id));

Looking at all the callers, noone seems to benefit from the return value being a JSFlatString.  Can we just have IdToString return a JSString* and keep this the same (actually, we could also remove the str->ensureFlat).

Comment 51

[Luke Wagner [:luke]]

(In reply to Luke Wagner [:luke] from comment #50)
> Next, two ideas:
> 1. could we have the invariant that any integer id is a JSInt32Atom?  That
> would make IdToString fast in the non-special case and I think work well
> with NativeIterator.

Uhhh, strike this idea, it is logically contradictory.

Comment 52

[Luke Wagner [:luke]]

Comment on attachment 653100 [details] [diff] [review]
v1

I'll clear the request while waiting for responses to questions in comment 50.  (To be clear, the patch looks nice, it's just important (for this and any other non-local optimization) to have a clear picture of what what it buys us.)

Comment 53

[Tom S [:evilpie]]

So some numbers. A single google search hits JSInt32String::value around 750 times. Most of these seem to be fore GetElement, too. I run the membuster benchmark, because this opens a lot of probably common pages, and I saw about 65,000 invocations.

I while back I asked mraleph about why they have this optimization in v8, and he also only came up with the for .. in pattern. https://twitter.com/evilpies/status/237982199092232192
Comment #45 and #46 have some numbers on how much this helps there.

I am currently trying to hunt down some other usages, but this isn't very easy, because I need to find hot uses and than try to find out how the look like in JavaScript.

Comment 54

[Tom S [:evilpie]]

http://gregor-wagner.com/tmp/mem has 420,935 JSInt32String::value calls.

Comment 55

[Luke Wagner [:luke]]

(In reply to Tom Schuster [:evilpie] from comment #53)
Ah, thanks for the investigation and sticking with it!

> I while back I asked mraleph about why they have this optimization in v8,
> and he also only came up with the for .. in pattern.
> https://twitter.com/evilpies/status/237982199092232192
> Comment #45 and #46 have some numbers on how much this helps there.

Oops, I missed them.  So, 2x faster with the appropriate IC.

(In reply to Tom Schuster [:evilpie] from comment #54)
> http://gregor-wagner.com/tmp/mem has 420,935 JSInt32String::value calls.

Well, I guess that means it shows up in real code, but not a ton.  That's seems good enough for me given the 2x for..in speedup.  Btw, with your IM ic patch, how do we compare to v8 on:
  for (var i in a)
    sum += a[i]
?

Next: do you see any speedup on SS/V8?  What is the before/after count of calls to ToNumberSlow (I think this is where all string->int conversions funnel through, is that right?)?

Lastly, a random note: I see this patch will optimize the GetElem path, but not SetElem.  (Man, the set-elem VM path is a mess; I wish we'd factor it more like GetElementOperation and pass the index as a Value.)

Comment 56

[Tom S [:evilpie]]

(In reply to Luke Wagner [:luke] from comment #55)
> (In reply to Tom Schuster [:evilpie] from comment #53)
> Ah, thanks for the investigation and sticking with it!
> 
> > I while back I asked mraleph about why they have this optimization in v8,
> > and he also only came up with the for .. in pattern.
> > https://twitter.com/evilpies/status/237982199092232192
> > Comment #45 and #46 have some numbers on how much this helps there.
> 
> Oops, I missed them.  So, 2x faster with the appropriate IC.
> 
> (In reply to Tom Schuster [:evilpie] from comment #54)
> > http://gregor-wagner.com/tmp/mem has 420,935 JSInt32String::value calls.
> 
> Well, I guess that means it shows up in real code, but not a ton.  That's
> seems good enough for me given the 2x for..in speedup.  Btw, with your IM ic
> patch, how do we compare to v8 on:
>   for (var i in a)
>     sum += a[i]
> ?
> 
Will look at this after I fixed the problem below.
> Next: do you see any speedup on SS/V8?  What is the before/after count of
> calls to ToNumberSlow (I think this is where all string->int conversions
> funnel through, is that right?)?
> 
I didn't see any speedup. But for some reason runs are pretty different on my machine. 
Good hunch with ToNumberSlow, it is actually invoked more often now. I am not sure yet why, will need to look into that. Hopefully it's nothing serious, but it did increase from ~35k to ~36k calls.
> Lastly, a random note: I see this patch will optimize the GetElem path, but
> not SetElem.  (Man, the set-elem VM path is a mess; I wish we'd factor it
> more like GetElementOperation and pass the index as a Value.)
You are right, going to fix this in a different bug.

Comment 57

[Tom S [:evilpie]]

I am not working on this.

Comment 58

[Nicholas Nethercote [inactive]]

(In reply to Tom Schuster [:evilpie] from comment #0)
> I think v8 has this, and Opera maybe, too.

Well, Opera definitely has it now, since it's just a Chromium clone...

Comment 59

[Tom S [:evilpie]]

I think I am going to try and get this fixed. For bug 1356315, I really want to have some quick way to get an index from a string key when enumerating an array with for..in. CacheIR is also able to convert strings to indexes, but needs a VM call for that.

Comment 60

[Tom S [:evilpie]]

Attachment: WIP

This actually seems to be a lot easier now, because we have enough bits in flags to store at least 16bits. Avoid the extra string type makes this a lot simpler.

We still have to look for places where we now should use getIndexValue, especially also inlining in the JITs. An obvious place is of course GetStringFromIndex callers. But we could also handle various other string-to-int operations in Ionmonkey, where we before didn't optimize at all.

Comment 61

[Tom S [:evilpie]]

Attachment: Cache index values in strings

This looks really good on try and I think this approach is quite simple for what it does.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=86b18fed7a17367b5b779b9881b400f94eafd20a&selectedJob=91924266
I had to include String.h in jsnum.h, but judging from the try result this was already happening everywhere but ARM.

Comment 62

[Tom S [:evilpie]]

Attachment: Cache index values in strings

Ups, qrefresh fail.

Comment 63

[Tom S [:evilpie]]

Attachment: Optimize call to GetIndexFromString in CacheIR

Comment 64

[Tom S [:evilpie]]

Attachment: Optimize call to GetIndexFromString in CacheIR

Comment 65

[Jan de Mooij [:jandem]]

Comment on attachment 8859561 [details] [diff] [review]
Cache index values in strings

Review of attachment 8859561 [details] [diff] [review]:
-----------------------------------------------------------------

Great to have this finally! \o/ There's one potential issue with atoms:

- Permanent atoms are shared with workers. I think the patch is fine because we set the index in StaticStrings::init and these are the only index permanent atoms AFAIK, but it would be nice to assert this (see below).

- Background parser threads also use atoms and races will confuse TSan. What if we do the following:

(1) Add a Maybe<uint32_t> argument to AtomizeAndCopyChars, then in AtomizeString pass str->getIndexValue() and in Int32ToAtom pass the int32 argument. Then we can set this on the atom while we have the atoms lock.

(2) Add an optional allowAtom (or something) argument to maybeInitializeIndex, then MOZ_ASSERT_IF(!allowAtom, !isAtom()); We can pass allowAtom = true in AtomizeAndCopyChars and StaticStrings::init where we know it's okay.

Unrelated: I think we can also check hasIndexValue/getIndexValue in num_parseInt and num_parseFloat? This will help bug 930540 and see also bug 840927 comment 7.

::: js/src/jsnum.cpp
@@ +1388,5 @@
>                        cbuf.dbuf && cbuf.dbuf == numStr);
>      }
>  
>      JSFlatString* s = NewStringCopyZ<allowGC>(cx, numStr);
> +    if (isBase10Int && i >= 0)

Nit: null-check |s| and return on OOM before this.

Comment 66

[Jan de Mooij [:jandem]]

Comment on attachment 8859574 [details] [diff] [review]
Optimize call to GetIndexFromString in CacheIR

Review of attachment 8859574 [details] [diff] [review]:
-----------------------------------------------------------------

Cool. Maybe we could also use this to optimize the StringToNumber OOL VM calls in CodeGenerator.cpp?

::: js/src/jit/MacroAssembler.cpp
@@ +1432,5 @@
> +  // Does not have a cached index value.
> +  branchTest32(Assembler::Zero, dest, Imm32(JSString::INDEX_VALUE_BIT), fail);
> +
> +  // Extract the index.
> +  rshiftPtr(Imm32(JSString::INDEX_VALUE_SHIFT), dest);

Nit: rshift32? Also the indentation in this method is off (tab or 2 spaces).

Comment 67

[Tom S [:evilpie]]

Good catch with the Atoms, I had not thought of that at all! Interestingly enough my old patches actually had the code in parseInt, I just added that back.

Comment 68

[Tom S [:evilpie]]

Attachment: v2 - Cache index values in strings

Thanks for the implementations tips, it really made this easier.

Comment 69

[Tom S [:evilpie]]

We need to figure out a good way to initialize the cached index value in the parser for code like `var x = "1000"`.

Comment 70

[Jan de Mooij [:jandem]]

Comment on attachment 8860390 [details] [diff] [review]
v2 - Cache index values in strings

Review of attachment 8860390 [details] [diff] [review]:
-----------------------------------------------------------------

Great \o/

Comment 71

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/a139ec75777a
Cache index values in JS strings. r=jandem
https://hg.mozilla.org/integration/mozilla-inbound/rev/18c3c29e52b4
Optimize calls to GetIndexFromString in CacheIR. r=jandem

Comment 72

[Iris Hsiao [:ihsiao]]

https://hg.mozilla.org/mozilla-central/rev/a139ec75777a
https://hg.mozilla.org/mozilla-central/rev/18c3c29e52b4