Closed Bug 568968 Opened 12 years ago Closed 12 years ago

Optimize XPC_WN_JSOp_ThisObject to not compute things it won't need

Categories

(Core :: XPConnect, defect)

x86
Linux
defect
Not set
normal

Tracking

()

RESOLVED FIXED
Tracking Status
blocking2.0 --- betaN+

People

(Reporter: mrbkap, Assigned: mrbkap)

References

(Blocks 1 open bug)

Details

Attachments

(1 obsolete file)

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.
Blocks: domperf
Attached patch Possible fix (obsolete) — Splinter Review
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: --- → ?
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
Closed: 12 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.