Closed Bug 287282 Opened 19 years ago Closed 16 years ago

Code Cleanup and Possible Perf Increase in xptiInterfaceInfoManager::BuildFileList

Categories

(Core :: XPCOM, defect)

x86
All
defect
Not set
trivial

Tracking

()

RESOLVED FIXED

People

(Reporter: Callek, Assigned: Callek)

Details

Attachments

(1 file, 2 obsolete files)

I was tracing through this, and thought it could be done a bit cleaner, Talked
with DBradley on IRC, the nsCOMPtr changes were suggested to me.
Attached patch Proposed Fix (obsolete) — Splinter Review
Dbradley will do perf tests on this, relatively untested by myself, did compile
test though.
Assignee: dougt → nobody
QA Contact: xpcom
I am still not easily able to perf-test any of you whom I just CCed able to do such or at least point me at someone who may be able to.

(From glancing this patch should still apply)
Attachment #178280 - Flags: review?(dbradley)
Refresh my memory, how is iterating the files backwards faster?
(In reply to comment #3)
> Refresh my memory, how is iterating the files backwards faster?
> 

The actual suspected increase is the InsertElementAt change, that is from moving PRUint32 count to be in a larger scope.

The iterating the paths backwards is from the desire to keep the actual resulting array order intact (incase the order matters elsewhere in code, my ability at the time; and my time now; do/did not allow me to investigate if the resulting array order was important, with this patch it doesn't change).

Comment on attachment 178280 [details] [diff] [review]
Proposed Fix

r=dbradley

Couldn't you just call AppendElement instead of InsertElementAt and then remove count all together?
Attachment #178280 - Flags: review?(dbradley) → review+
Attached patch Patch for checkin (obsolete) — Splinter Review
Fixed comments per IRC discussion, and use AppendElement as suggested.

Will check in within a few days.
Assignee: nobody → bugspam.Callek
Status: NEW → ASSIGNED
left out one change in attachment 334039 [details] [diff] [review]
Attachment #178280 - Attachment is obsolete: true
Attachment #334039 - Attachment is obsolete: true
$ hg push
pushing to ssh://hg.mozilla.org/mozilla-central/
searching for changes
remote: adding changesets
remote: adding manifests
remote: adding file changes
remote: added 1 changesets with 1 changes to 1 files

$ hg tip -v
changeset:   18359:49ed8663765f
tag:         tip
user:        Justin Wood <Callek@gmail.com>
date:        Sun Aug 24 21:09:23 2008 -0400
files:       xpcom/reflect/xptinfo/src/xptiInterfaceInfoManager.cpp
description:
Bug 287282, Code Cleanup and Possible Perf Increase in xptiInterfaceInfoManager::BuildFileList
r=dbradley

Sadly though, preliminary perf numbers suggest there is no actual improvement here.

Leaving in per IRC discussion with dbradley, will comment here if anything worthwhile is noticed.
Status: ASSIGNED → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
I'm no native speaker, but isn't "occurr" spelled with one r?

https://hg.mozilla.org/mozilla-central/rev/49ed8663765f#l14

It's a nit only and I don't want to open a new bug for this
It is, thank you; can you please add that version of the misspelling in a comment on Bug 106386.
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: