Closed
Bug 914132
Opened 11 years ago
Closed 11 years ago
IonMonkey: typeof improvements
Categories
(Core :: JavaScript Engine, defect)
Core
JavaScript Engine
Tracking
()
RESOLVED
FIXED
mozilla26
People
(Reporter: jandem, Assigned: jandem)
References
Details
Attachments
(4 files)
1.30 KB,
patch
|
evilpies
:
review+
|
Details | Diff | Splinter Review |
9.69 KB,
patch
|
evilpies
:
review+
|
Details | Diff | Splinter Review |
7.35 KB,
patch
|
bhackett1024
:
review+
|
Details | Diff | Splinter Review |
895 bytes,
patch
|
bhackett1024
:
review+
|
Details | Diff | Splinter Review |
No description provided.
Assignee | ||
Comment 1•11 years ago
|
||
Bug 839027 removed the typeof hook, so typeof is no longer effectful.
This improves test10b in bug 606648 from 584 to 505 ms.
Attachment #801555 -
Flags: review?(evilpies)
Assignee | ||
Comment 2•11 years ago
|
||
The TypeOf functions are infallible now, so this patch removes an unnecessary root and cx arguments. Also adds TypeOfObjectOperation and uses callWithABI instead of the more heavyweight callVM machinery to call it.
On top of the previous patch, we're now down to 298 ms, d8 is at 375 ms, but there's more we can do.
Attachment #801578 -
Flags: review?(evilpies)
Attachment #801555 -
Flags: review?(evilpies) → review+
Comment on attachment 801578 [details] [diff] [review]
Part 2 - Use infallible calls
Review of attachment 801578 [details] [diff] [review]:
-----------------------------------------------------------------
::: js/src/jit/CodeGenerator.cpp
@@ +6422,5 @@
> +
> + saveVolatile(output);
> + masm.setupUnalignedABICall(2, output);
> + masm.passABIArg(obj);
> + masm.loadJSContext(output);
You can just use
movePtr(ImmWord(GetIonContext()->runtime), output);
after you made the other changes. One load less ;)
::: js/src/vm/Interpreter-inl.h
@@ +469,5 @@
> return GetObjectElementOperation(cx, op, obj, isObject, rref, res);
> }
>
> static JS_ALWAYS_INLINE JSString *
> +TypeOfOperation(const Value &v, JSContext *cx)
These two functions could take a JSRuntime pointer. You then we could try removing the TypeName version with JSContext as well. This depends if that make a lot more code worse or better.
::: js/src/vm/Interpreter.h
@@ +310,5 @@
> extern JSType
> +TypeOfObject(JSObject *obj);
> +
> +extern JSType
> +TypeOfValue(const Value &v);
I feel like these two functions should live in jsobj(inlines).h
Attachment #801578 -
Flags: review?(evilpies) → review+
Assignee | ||
Comment 4•11 years ago
|
||
Don't use an OOL call for "typeof object" if we know the operand is not callable and does not emulate undefined.
This brings our time down to 139 ms (from 584 ms without these patches), just 12 ms slower than the version without typeof (bug 606648 comment 15).
Attachment #801609 -
Flags: review?(bhackett1024)
Assignee | ||
Updated•11 years ago
|
Attachment #801609 -
Flags: review?(bhackett1024)
Assignee | ||
Updated•11 years ago
|
Attachment #801609 -
Flags: review?(bhackett1024)
Updated•11 years ago
|
Attachment #801609 -
Flags: review?(bhackett1024) → review+
Assignee | ||
Comment 5•11 years ago
|
||
Forgot this part yesterday. This brings our time down to 97 ms, faster than the version without typeof.
Attachment #802163 -
Flags: review?(bhackett1024)
Updated•11 years ago
|
Attachment #802163 -
Flags: review?(bhackett1024) → review+
Assignee | ||
Comment 6•11 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/5fd5f21d4663
https://hg.mozilla.org/integration/mozilla-inbound/rev/61824642543a
https://hg.mozilla.org/integration/mozilla-inbound/rev/8c452ca6d416
https://hg.mozilla.org/integration/mozilla-inbound/rev/e112a8245e29
(In reply to Tom Schuster [:evilpie] from comment #3)
> These two functions could take a JSRuntime pointer. You then we could try
> removing the TypeName version with JSContext as well. This depends if that
> make a lot more code worse or better.
Good idea, thanks! After doing that there was only one caller of TypeName(.., JSContext), so I changed it to pass cx->runtime() and removed the TypeName overload that takes a JSContext.
> ::: js/src/vm/Interpreter.h
> I feel like these two functions should live in jsobj(inlines).h
Moving them to jsobjinlines.h requires an extra #include of jsboolinlines.h for EmulatesUndefined... I kept them in Interpreter* for now so that they can be inlined in the interpreter loop (without requiring link-time optimization etc).
Comment 7•11 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/5fd5f21d4663
https://hg.mozilla.org/mozilla-central/rev/61824642543a
https://hg.mozilla.org/mozilla-central/rev/8c452ca6d416
https://hg.mozilla.org/mozilla-central/rev/e112a8245e29
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla26
You need to log in
before you can comment on or make changes to this bug.
Description
•