On 2013/11/12 07:09:12, Sven Panne wrote:
Addressed feedback. Need to look into some regressions in SunSpider's
string-base64 and string-validate-input benchmarks. Basically they seem to
measure Math.random's performance, which you can of course guess from the
benchmark's names and is of utmost importance for real-world
JavaScript... :-P
https://codereview.chromium.org/68723002/diff/1/src/assembler.cc
File src/assembler.cc (left):
https://codereview.chromium.org/68723002/diff/1/src/assembler.cc#oldcode1064
src/assembler.cc:1064: ExternalReference
ExternalReference::random_uint32_function(
On 2013/11/11 14:11:12, Michael Starzinger wrote:
> Please also remove the declaration in the assembler.h file.
Done.
https://codereview.chromium.org/68723002/diff/1/src/bootstrapper.cc
File src/bootstrapper.cc (right):
https://codereview.chromium.org/68723002/diff/1/src/bootstrapper.cc#newcode2644
src/bootstrapper.cc:2644: factory()->NewHeapNumber(state[0]),
On 2013/11/11 14:11:12, Michael Starzinger wrote:
> Better use Factory::NewNumberFromUint here and in the call below.
Done.
https://codereview.chromium.org/68723002/diff/1/src/math.js
File src/math.js (right):
https://codereview.chromium.org/68723002/diff/1/src/math.js#newcode180
src/math.js:180: random0 = r0;
On 2013/11/11 14:11:12, Michael Starzinger wrote:
> I am not sure about the sequence of the initialization here. Can we make
sure
> that "random0" and "random1" are bound to global variables by explicitly
> declaring them at the top-level a few lines above? Thereby it is more
obvious
to
> where these two variables are bound.
Done.
I have two things in mind that could improve performance, both of which also
apply to the javascript implementation of Math.sin
- Math.random is no longer inlined with this CL, which is very important for
performance. Inlining builtins seems to be a tricky business, and so far, we
manually chose them for inlining via TryInlining{Builtin|Method}Call. Simply
using our inlining heuristics to also automatically choose builtins for
inlining
does not work out well. With my math.sin CL, I'm introducing a
%SetInlineBuiltinFlag, mirroring %SetNativeFlag, as compiler hint to force
inline the specified function. This is no different than what we do with
TryInlining{Builtin|Method}Call anyways.
- You are using division by 0x100000000 at the end, which is slightly more
expensive than multiplying with the inverse (I checked). Not sure if the
result
is exactly the same, though (I think it is).
https://codereview.chromium.org/68723002/
--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/groups/opt_out.
