Optimize XPC_WN_JSOp_ThisObject to not compute things it won't need

RESOLVED FIXED

Status

()

RESOLVED FIXED
9 years ago
8 years ago

People

(Reporter: mrbkap, Assigned: mrbkap)

Tracking

(Blocks: 1 bug)

Trunk
x86
Linux
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(blocking2.0 betaN+)

Details

Attachments

(1 obsolete attachment)

Currently, the thisObject hook in XPConnect computes a bunch of stuff up front in order to call nsXPConnect::GetWrapperForObject. This includes getting the current subject principal (amongst other things). We should delay computing stuff as much as we possibly can. This should allow us to call the ThisObject hook without falling off trace.
Created attachment 448151 [details] [diff] [review]
Possible fix

I haven't tested this at all (though I know the browser comes up). It isn't "clean" in that all it does is add another fast path, but I have another patch still in the works that makes nsXPConnect::GetWrapperForObject go away (in favor of XPCWrappedNativeScope::GetWrapperFor, which doesn't knock us off trace and doesn't call into the security manager at all).

I'll tryserver this, make sure it's doing what I want it to do, and request review.
Assignee: nobody → mrbkap
Status: NEW → ASSIGNED
Attachment #448151 - Flags: review?(jst)
This doesn't apply as is, I'm guessing it goes on top of some of your other patches, I think I've even seen the change that would make this apply, but don't have a bug number for it...
This is a major slowdown when we hit it, which tends to be in poorly-written benchmarks (most of them).  We need to get this fixed....

Do jsreftests have access to jitstats?  Can we detect deep bails in jitstats?  As in, can we add a test for this?
blocking2.0: --- → ?

Updated

8 years ago
blocking2.0: ? → betaN+
Is this really a blocker?
This is fixed by the compartments patch.
So should this depend on bug 558861?
bug 580128 is closer.
Depends on: 580128
Attachment #448151 - Attachment is obsolete: true
Attachment #448151 - Flags: review?(jst)
Fixed by brain transplants.
Status: ASSIGNED → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.