Code Cleanup and Possible Perf Increase in xptiInterfaceInfoManager::BuildFileList

RESOLVED FIXED

Status

()

Core
XPCOM
--
trivial
RESOLVED FIXED
13 years ago
10 years ago

People

(Reporter: Callek, Assigned: Callek)

Tracking

Trunk
x86
All
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Assignee)

Description

13 years ago
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.
(Assignee)

Comment 1

13 years ago
Created attachment 178280 [details] [diff] [review]
Proposed Fix

Dbradley will do perf tests on this, relatively untested by myself, did compile
test though.
Assignee: dougt → nobody
QA Contact: xpcom
(Assignee)

Comment 2

10 years ago
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)

Updated

10 years ago
Attachment #178280 - Flags: review?(dbradley)

Comment 3

10 years ago
Refresh my memory, how is iterating the files backwards faster?
(Assignee)

Comment 4

10 years ago
(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 5

10 years ago
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+
(Assignee)

Comment 6

10 years ago
Created attachment 334039 [details] [diff] [review]
Patch for checkin

Fixed comments per IRC discussion, and use AppendElement as suggested.

Will check in within a few days.
Assignee: nobody → bugspam.Callek
Status: NEW → ASSIGNED
(Assignee)

Comment 7

10 years ago
Created attachment 334176 [details] [diff] [review]
actual patch for checkin

left out one change in attachment 334039 [details] [diff] [review]
Attachment #178280 - Attachment is obsolete: true
Attachment #334039 - Attachment is obsolete: true
(Assignee)

Comment 8

10 years ago
$ 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
Last Resolved: 10 years ago
Resolution: --- → FIXED

Comment 9

10 years ago
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
(Assignee)

Comment 10

10 years ago
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.