David Anderson [:dvander] - inactive, e-mail if emergency Assignee
	Description • 16 years ago

http://users.alliedmods.net/~dvander/tm_chrome_test.jpg

We're hurting on parseInt and concat.

	Andreas Gal :gal
	Comment 1 • 16 years ago

Are we aborting trace?

	David Anderson [:dvander] - inactive, e-mail if emergency Assignee
	Comment 2 • 16 years ago

on parseInt() we don't trace because a number is being passed instead of a string.

	Andreas Gal :gal
	Comment 3 • 16 years ago

Thats awesome. Can you whip up a patch to special-case this? (no-op). We will be very fast on this one ;)

	David Anderson [:dvander] - inactive, e-mail if emergency Assignee
	Comment 4 • 16 years ago

Quick patch, from IRC there was a sort-of consensus that parseInt on a double became a Math.floor() with checks for infinite values (which have to become NaN).  With this my test machine went from 3877ms to 46ms (type specialization rocks!)

Will do a separate patch for parseInt(int) as I'd like to make a slightly more involved structure for no-op natives.

	Andreas Gal :gal
	Comment 5 • 16 years ago

Comment on attachment 337266 [details] [diff] [review]
proposed patch for parseInt(double)

The opening brace after the if is desperately calling to be moved one line up.

	Brendan Eich [:brendan]
	Comment 6 • 16 years ago

(In reply to comment #5)
> (From update of attachment 337266 [details] [diff] [review])
> The opening brace after the if is desperately calling to be moved one line up.

And then apply the brace minimization filter ;-).

/be

	Andreas Gal :gal
	Comment 7 • 16 years ago

Or how about this:

return JS_DOUBLE_IS_FINITE(d) ? js_NaN : floor(d);

/me runs and ducks before shaver sees this.

	Brendan Eich [:brendan]
	Comment 8 • 16 years ago

Yeah, then the ?: rewrite filter -- I'm not kidding, one liners like this are better in my book.

/be

	Mike Shaver (:shaver -- probably not reading bugmail closely)
	Comment 9 • 16 years ago

See also bug 453402; as in that bug, I think we want different bugs for the different slow cases.

(I think Andreas' one-liner, for all its terse glory, is perhaps backwards?)

	Boris Zbarsky [:bzbarsky]
	Comment 10 • 16 years ago

For what it's worth, the concat benchmark is basically spending all its time in/under js_ConcatStrings.  Some JS_malloc, some js_NewString, some js_strncpy.  That pretty much accounts for all of it.

Not sure what v8 is doing here other than perhaps realizing that the whole thing is a no-op...

	Mike Shaver (:shaver -- probably not reading bugmail closely)
	Comment 11 • 16 years ago

Suspect that they're refcounting strings, and/or using a multi-segment string implementation internally.  Haven't had time to check, though. :)

	Boris Zbarsky [:bzbarsky]
	Comment 12 • 16 years ago

Yeah....  I spent some time poking through their code, but can't even find the string implementation, so much.  There's the string.js thing which they presumably jit and all, but I can't find the string backend.

	Mike Shaver (:shaver -- probably not reading bugmail closely)
	Comment 13 • 16 years ago

runtime.cc's StringAdd leads to Heap::AllocateConsString for not-tiny strings, indeed.

	Alfred Kayser
	Comment 14 • 16 years ago

For the string concat issue, the test concats two static strings, for which I expect a smart JIT compiler to detect this situation, optimizing the add of two constants into one new constant.

A long time, I developed an interpreter (DScheme) that did this kind of 'inline' formula optimization when translating a formula to a bytecode stream, so this should also be possible for JavaScript.

Can this be taken up in a separate bug?

	Boris Zbarsky [:bzbarsky]
	Comment 15 • 16 years ago

We already jit into the string concatenation built-in.  The time in that test is taken purely by allocating the new string object and copying in the data.

	Alfred Kayser
	Comment 16 • 16 years ago

See bug 453402 for the string concat issue.

	David Anderson [:dvander] - inactive, e-mail if emergency Assignee
	Comment 17 • 16 years ago

Fix for parseInt(double) was pushed to m-c with last night's merge.