Closed Bug 515496 (blazinglyfastthis) Opened 12 years ago Closed 12 years ago

Eliminate extra security check when computing this (in js_ComputeGlobalThis)

Categories

(Core :: JavaScript Engine, defect)

x86
Linux
defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: mrbkap, Assigned: mrbkap)

References

Details

(Whiteboard: fixed-in-tracemonkey)

Attachments

(1 file)

Currently, computing this is very expensive, even in the interpreter and tracer. We end up calling into a hook that needs to look at the stack and make all sorts of bizarre decisions. In reality, when we're in the interpreter, we never need to do this. All 'this' computations that happen in the interpreter can use the wrapper for their scope (note that assertion relies on full wrapperization, which is not quite here yet, thanks to evalInSandbox).

The overall proposal is to tie JSCLASS_GLOBAL_FLAGS to a slot on the global object that contains the "stunt this object" and eagerly use that when calling functions.

There is one case where we can't directly use the stunt this object, which is on entry from native code (via the API) in that case, we should be able to simply call the thisObject hook or something to find the right this object to use.
> All 'this' computations that happen in the interpreter
> can use the wrapper for their scope

Which wrapper? and what does "scope" mean here?

> (note that assertion relies on full wrapperization, which is not quite
> here yet, thanks to evalInSandbox).

How much effort would this be?

In bug 513160 comment 8 I proposed computing `this` eagerly rather than lazily, and caching those computations via the property cache. But certainly if the computation can be made BLAZINGLY FAST, that's even better.
Blocks: 522890
Depends on: 533600, 533592, 533596
Alias: blazinglyfastthis
Depends on: 542428
Blocks: strictThis
Attached patch PatchSplinter Review
So, a couple of things got in the way of really BLAZINGLY fast 'this'. In particular, I was really hoping to avoid having to call any hooks for 'this' wrapping in the common case. This doesn't quite get us there, but it gets rid of the security check that is now unnecessary.

In order to get rid of the wrapper *hook*, we need to group wrappers by origin, not scope as we do now. That patch is coming (for other, more awesome reasons as well).
Assignee: general → mrbkap
Status: NEW → ASSIGNED
Attachment #433450 - Flags: review?(jorendorff)
Comment on attachment 433450 [details] [diff] [review]
Patch

>-         * When the above FIXME is made fixed, the whole GC reachable frame
>-         * mechanism can be removed as well.

I think this is true. The only other use of JSGCReachableFrame is in jsdbgapi.cpp:JS_GetFrameThis, and I think that can be removed.

Otherwise this patch looks great.
Attachment #433450 - Flags: review?(jorendorff) → review+
Another note: It seems better to inline JS_GetGlobalForObject or at least avoid the expense of calling the public API. A JSObject method might be nice.
I pushed this to tm. mrbkap is busy.

http://hg.mozilla.org/tracemonkey/rev/9851a9b1647f
Whiteboard: fixed-in-tracemonkey
Adjusting the summary to reflect what actually happened here.

I'm leaving the (blazinglyfastthis) alias for now. I hope that alias can be re-assigned to a followup bug soon!
Summary: Make computing 'this' be BLAZINGLY FAST → Eliminate extra security check when computing this (in js_ComputeGlobalThis)
Blocks: 556315
http://hg.mozilla.org/mozilla-central/rev/9851a9b1647f
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Is JS_THIS_OBJECT infallible now?  Should bug 498543 be closed?
You need to log in before you can comment on or make changes to this bug.