Closed
Bug 80364
Opened 24 years ago
Closed 6 years ago
xpconnect could save a little work in iterating the foundDependentParam case
Categories
(Core :: XPConnect, defect, P3)
Core
XPConnect
Tracking
()
RESOLVED
INACTIVE
mozilla1.7alpha
People
(Reporter: jband_mozilla, Assigned: dbradley)
Details
Attachments
(1 file)
shaver points out...
>
> + if(foundDependentParam)
> + {
> + for(i = 0; i < paramCount; i++)
>
> So instead of just using a boolean, you could store the index of the first
> dependent param, and save yourself some iterations, no? (Not that the
> !IsDependent cases are very expensive, of course.)
>
> + for(i = 0; i < paramCount; i++)
> + {
> + const nsXPTParamInfo& paramInfo = methodInfo->GetParam(i);
> + if(!paramInfo.IsOut() && !paramInfo.IsDipper())
> + continue;
>
> Likewise for storing the first out/dipper param index and starting there?
>
> +done:
> + // iterate through the params (again!) and clean up
> + // any alloc'd stuff and release wrappers of params
> + if(dispatchParams)
> + {
>
> Ditto for storing the first param for cleanup?
This is pretty trivial but a valid point.
Assignee | ||
Updated•24 years ago
|
Status: NEW → ASSIGNED
Assignee | ||
Updated•23 years ago
|
Target Milestone: --- → mozilla1.0
Assignee | ||
Comment 1•23 years ago
|
||
Moving up for performance/footprint focus
Target Milestone: mozilla1.0 → mozilla0.9.7
Assignee | ||
Updated•23 years ago
|
Priority: -- → P3
Assignee | ||
Comment 2•23 years ago
|
||
The benefit of this is pretty trivial moving out past 1.0. I'm attaching a patch
that I mucked around with. It eliminates the use of the dependent param loop. I
had three goals, speed things up, break CallMethod into smaller pieces, and
reduce code size. I was able to achieve two of the three, code size didn't
decrease, but increased slightly. I was hoping with the removal of the loop code
I'd see a small gain.
Target Milestone: mozilla0.9.7 → mozilla1.0.1
Assignee | ||
Comment 3•23 years ago
|
||
Reporter | ||
Comment 4•23 years ago
|
||
dbradley: This bug is about suggesting an optimization with near zero value (few
methods have dependent params AND many params AND would be sped up in any
measurable way by skipping iterations in a quickly shorcircuited loop). Yet, you
are suggesting a 1200 line patch that completely reorganizes the code and
introduces an unknown number of potential bugs. e.g. check the meaning of
IsDependent()...
http://lxr.mozilla.org/seamonkey/source/xpcom/reflect/xptinfo/public/xptinfo.h#140
I just don't buy this at all. I realize that this was probably just (as much as
anyting) an experiment. I can understand your possible dismay at the complex
top-down method that has evolved here. But, rewrites have cost and cost requires
payoff. I'm not seeing that here.
Reporter | ||
Updated•23 years ago
|
Attachment #60489 -
Flags: needs-work+
Assignee | ||
Comment 5•23 years ago
|
||
Yes, it was weekend experiment. I posted it for future reference as something to
think about if this code ever went through a major overhaul. I didn't mean for
anyone to waste time on it, I should have made that intention explicit in the
comment.
Assignee | ||
Comment 6•23 years ago
|
||
Retargetting to the proper post 1.0 milestone
Target Milestone: mozilla1.0.1 → mozilla1.2
Assignee | ||
Comment 7•22 years ago
|
||
Moving out to 1.3. If this needs to be in before 1.3 please comment.
Target Milestone: mozilla1.2alpha → mozilla1.3alpha
Assignee | ||
Comment 8•22 years ago
|
||
Moving to 1.4 Alpha
Target Milestone: mozilla1.3alpha → mozilla1.4alpha
Assignee | ||
Updated•21 years ago
|
Target Milestone: mozilla1.5alpha → mozilla1.5beta
Assignee | ||
Comment 10•21 years ago
|
||
Moving out, speak up if you believe this needs to be considered for 1.5b
Target Milestone: mozilla1.5beta → mozilla1.6alpha
Updated•18 years ago
|
QA Contact: pschwartau → xpconnect
Comment 12•6 years ago
|
||
Per policy at https://wiki.mozilla.org/Bug_Triage/Projects/Bug_Handling/Bug_Husbandry#Inactive_Bugs. If this bug is not an enhancement request or a bug not present in a supported release of Firefox, then it may be reopened.
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → INACTIVE
You need to log in
before you can comment on or make changes to this bug.
Description
•