Should optimize entry order in QI to hit frequent IIDs first

RESOLVED WONTFIX

Status

()

Core
XPCOM
P5
normal
RESOLVED WONTFIX
17 years ago
4 years ago

People

(Reporter: Simon Fraser, Unassigned)

Tracking

({helpwanted, perf})

Trunk
Future
helpwanted, perf
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(4 attachments)

(Reporter)

Description

17 years ago
Talking to scc and others, we hit upon a cheap optimization that should reduce 
the amount of time spent doing QueryInterface, and that is to ensure that QI 
implementations test for the frequent IIDs first. I'll attach some code, and some 
test results.
(Reporter)

Comment 1

17 years ago
Created attachment 27337 [details]
Raw QI frequency data
(Reporter)

Comment 2

17 years ago
The data show, for each QI, how many times the QI was called with a given IID. No 
data is given for whether that IID is matched or not. For example:

QueryInterface at in file: nsSupportsPrimitives.cpp at line: 169
{d79dc970-4a1c-11d3-9890-006008962422}  25
{00000000-0000-0000-c000-000000000046}  9

This QI was called 34 times, 25  with {d79dc970-4a1c-11d3-9890-006008962422}, and 
9 with {00000000-0000-0000-c000-000000000046}

IIDs are printed in a first-found order; they are not sorted in any way.
(Reporter)

Comment 3

17 years ago
In the run for which I collected data, I ran the page loading test once through, 
typed in a bugzilla entry textarea, opened composer and typed a bit, opened 
mailnews and loaded 2 IMAP folders, and made a new message then typed.
(Reporter)

Comment 4

17 years ago
That data is incomplete for two reasons:
1. Lots of classes don't use the macros:
   <http://lxr.mozilla.org/seamonkey/search?string=%3A%3AQueryInterface>

2. I failed to patch the old-style QI macro.

More data coming...

Comment 5

17 years ago
setting to moz0.9.1
Priority: -- → P3
Target Milestone: --- → mozilla0.9.1
(Reporter)

Comment 6

17 years ago
Created attachment 27488 [details] [diff] [review]
Code changes to generate data
(Reporter)

Comment 7

17 years ago
Created attachment 27489 [details]
More complete QI data
(Reporter)

Comment 8

17 years ago
The QI data I just attached are more complete. I went through the entire tree, 
searching for QI implementations that don't use the macros, and either macroized 
them, or added hooks to call the instrumentation class. (I didn't change any QIs 
in mailnews). I can't guarantee that I got all the QIs, but I got most of them.

The data were generated as before (once through the page loader, some typing in 
composer, a couple of large IMAP mail folders, messing around in bookmarks 
window).
(Reporter)

Comment 9

17 years ago
Created attachment 27495 [details]
Annotated list of most common QIs
(Reporter)

Comment 10

17 years ago
The latest attachment shows the QIs which happen most frequently (picked 
according to those that show big counts), with most of the IIDs annotated with 
the interface name.

This data should be used as input to re-ordering QIs for maximum performance. At 
first glance, the following QIs could benefit from reordering:

nsGlobalWindow
nsMenuItem/nsMenu (Mac implementations)
nsImageContextAsync (if libpr0n doesn't obsolete it)
nsDocShell/nsWebShell
nsWebShellWindow

Updated

17 years ago
Blocks: 71874
(Reporter)

Updated

17 years ago
Keywords: helpwanted, perf
(Reporter)

Updated

17 years ago
Target Milestone: mozilla0.9.1 → mozilla1.0

Comment 11

17 years ago
Changing platform
Hardware: All → Macintosh

Updated

17 years ago
OS: Mac System 8.5 → All
Hardware: Macintosh → All

Comment 12

16 years ago
Bugs targeted at mozilla1.0 without the mozilla1.0 keyword moved to mozilla1.0.1 
(you can query for this string to delete spam or retrieve the list of bugs I've 
moved)
Target Milestone: mozilla1.0 → mozilla1.0.1

Comment 13

16 years ago
don't move bugs that are in the 1.0 dependency tree. sorry.
Target Milestone: mozilla1.0.1 → mozilla1.0
(Reporter)

Comment 14

16 years ago
1.2
Status: NEW → ASSIGNED
Target Milestone: mozilla1.0 → mozilla1.2

Comment 15

16 years ago
What's the expected gain here?
(Reporter)

Comment 16

16 years ago
Pretty small, probably. Maybe there are a few hotspots.
(Reporter)

Updated

15 years ago
Target Milestone: mozilla1.2alpha → mozilla1.5alpha
(Reporter)

Updated

12 years ago
Assignee: sfraser_bugs → bzbarsky
Status: ASSIGNED → NEW
I'm only looking at this for DOM nodes, really...  Those are where QI misses suck a lot (because of the XBL dependencies).
Assignee: bzbarsky → dougt
QA Contact: kandrot → xpcom

Comment 18

10 years ago
mass reassigning to nobody.
Assignee: dougt → nobody

Comment 19

9 years ago
From the table-driven QI bugs, IID misses are pretty common and take up the large majority of the time, and the table makes the loop really tight. I don't think we'll win more by microoptimizing this.
Status: NEW → RESOLVED
Last Resolved: 9 years ago
Priority: P3 → P5
Resolution: --- → WONTFIX
Target Milestone: mozilla1.5alpha → Future
You need to log in before you can comment on or make changes to this bug.