We could possibly shortcut the data loads in nsID::Equals by checking whether the same object is being compared. Especially now that we have libxul, I believe that most instances of the same IID point to the same object (nsIFoo::COMTypeInfo<int>::kDummy) and most comparisons would be shortcuts. This suggestion comes from bug 165975... it may not be worth it, so I'd like to test-land it at some quiet time to get perf nubmers.
Attachment #295511 - Flags: review?(dbaron)
i did some quick instrumentation of nsID::Equals() for startup and pageload of gmail.com. "nsID ==" means the struct contents compared equal, "refs ==" means the refs were equal, and note that the data is distributed about the same for both startup and pageload considered separately, so this should be pretty representative: refs == | 0 | 1 nsID == --------------------|------------------- 0 | 2913022 (79.9%) | 0 1 | 257829 ( 7.1%) | 472958 (13.0%) so this test would benefit 13.0% of cases by saving three dword compares, and penalize 79.9 + 7.1 = 87.0% of cases by adding one dword compare. (previously, the 79.9% case would likely bail on the first dword compare.) perhaps it's worth shifting the ref test to after the m0 test, so we win back that 79.9% in the common case? then you're talking 13.0% vs 7.1% which is more likely to be a net win. this argument ignores the cost of loading m0 etc from memory, though, which may be large... then again if all these IID's are sitting in the same static data section in libxul, maybe there's some locality and it's not? the same problem cropped up in bug 165975, namely that the common case by far is inequality, so MMX/SSE and clever tricks don't buy much when the hurdle to beat is just one dword comparison. :/
yes, when you're running this method (inlined) both reference pointers should be in registers already, so comparing them is really fast compared to any memory loads... I think we need to just check this in to test it practice.
Comment on attachment 295511 [details] [diff] [review] compare references before continuing, rev. 1 r=dbaron, and bonus points for removing the unneeded PRBool cast the line before
Attachment #295511 - Flags: review?(dbaron) → review+
I test-landed this late last week and there was no appreciable perf difference. I just backed it out and I'm going to close this WONTFIX.
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.