Closed Bug 564575 Opened 12 years ago Closed 12 years ago

[FIX]Use iteration, not recursion, for registering/unregistering named items

Categories

(Core :: DOM: Core & HTML, defect)

x86
macOS
defect
Not set
normal

Tracking

()

RESOLVED WONTFIX

People

(Reporter: bzbarsky, Assigned: bzbarsky)

References

(Blocks 1 open bug)

Details

Attachments

(2 files)

Attached file Testcase
As the summary says.

This patch wins us about 6-7% on the attached testcase; still a bit behind tip webkit (which started out 15% ahead of us).
Attached patch Like soSplinter Review
Attachment #444234 - Flags: review?(jst)
Would it make sense to have a GetFirstElement (inline, returns this if IsElement is true or calls GetNextElement) and GetNextElement (inline, calls GetNextNode until IsElement is true)? I don't have data, but it seems like that would be a common thing to want to do?
Comment on attachment 444234 [details] [diff] [review]
Like so

> Would it make sense to have a GetFirstElement (inline, returns this if
> IsElement is true or calls GetNextElement) and GetNextElement (inline, calls
> GetNextNode until IsElement is true)? 

Probably, yes.  I'll roll that into the patch in bug 564435.

This patch is about to become irrelevant when bug 564863 lands.
Attachment #444234 - Flags: review?(jst)
Depends on: 564863
(In reply to comment #2)
> Would it make sense to have a GetFirstElement (inline, returns this if
> IsElement is true or calls GetNextElement) and GetNextElement (inline, calls
> GetNextNode until IsElement is true)? I don't have data, but it seems like that
> would be a common thing to want to do?

Ooh, i like this idea. Don't think it'll speed up anything, but it should make for cleaner code through more code reuse.
I just tried that in the patch for bug 564435.  On the testcase in that bug, it makes the code somewhat slower (about 5% or so), presumably due to the extra loop and corresponding null-checks.  Core cleanliness is not obviously much better than it used to be, due to having to start loops with GetFirstElement/GetNextElement instead of whatever the actual object you have is.

I filed bug 564883 on adding that api, if we decide to do it.
Wontfix, per bug 564863.
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → WONTFIX
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.