Summary: | Poor performance on crazy DOM raytracer | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | David Smith <catfish.man> | ||||||||
Component: | DOM | Assignee: | Nobody <webkit-unassigned> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | ddkilzer | ||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||
Version: | 523.x (Safari 3) | ||||||||||
Hardware: | Mac | ||||||||||
OS: | OS X 10.4 | ||||||||||
URL: | http://nontroppo.org/timer/progressive_raytracer.html | ||||||||||
Attachments: |
|
Description
David Smith
2007-09-06 00:18:15 PDT
Created attachment 16205 [details]
Speeds up the "full render" from >400 seconds to ~45 seconds for me
Thanks to everyone on irc for their suggestions and prodding :) Particularly the suggestion of using ListHashSet from Maciej.
Created attachment 16206 [details]
Fix my email address, and remove a stray formatting change
Minor tweaks, sorry for the spam.
This seems like a gratuitous change: - for ( ; (r = it.current()); ++it) { + for ( ; (r = it.current()); ++it ) { There's no need to use new here, you can just make the Vector a local variable: + Vector<RenderObject*>* deadObjects = new Vector<RenderObject*>(); In fact, it might be worthwhile giving it an inline capacity to handle the common case effeciently. Something like: Vector<RenderObject*, 16> deadObjects; r- to address these points. Created attachment 16229 [details] Addresses review comments >This seems like a gratuitous change: > >- for ( ; (r = it.current()); ++it) { >+ for ( ; (r = it.current()); ++it ) { Oops, leftover from a find & replace style fixup. Fixed. >There's no need to use new here, you can just make the Vector a local variable: > >+ Vector<RenderObject*>* deadObjects = new Vector<RenderObject*>(); > >In fact, it might be worthwhile giving it an inline capacity to handle the >common case effeciently. Something like: > >Vector<RenderObject*, 16> deadObjects; Fixed. Comment on attachment 16229 [details]
Addresses review comments
r=me
|