WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
126525
Move function calls outside loop in dom
https://bugs.webkit.org/show_bug.cgi?id=126525
Summary
Move function calls outside loop in dom
Laszlo Vidacs
Reported
2014-01-06 07:28:03 PST
length() method is called in each iteration in the for loop in dom (DOMImplementation.cpp, ElementData.cpp, EventContext.cpp), similarly to
https://bugs.webkit.org/show_bug.cgi?id=125916
Attachments
Patch
(3.68 KB, patch)
2014-01-06 07:37 PST
,
Laszlo Vidacs
no flags
Details
Formatted Diff
Diff
Patch
(3.02 KB, patch)
2014-02-12 01:42 PST
,
Laszlo Vidacs
no flags
Details
Formatted Diff
Diff
Patch
(3.14 KB, patch)
2014-02-24 06:10 PST
,
Laszlo Vidacs
no flags
Details
Formatted Diff
Diff
Show Obsolete
(2)
View All
Add attachment
proposed patch, testcase, etc.
Laszlo Vidacs
Comment 1
2014-01-06 07:37:53 PST
Created
attachment 220435
[details]
Patch
Alexey Proskuryakov
Comment 2
2014-01-06 09:37:38 PST
Comment on
attachment 220435
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=220435&action=review
> Source/WebCore/ChangeLog:8 > + Do not call length() in each iteration.
How did you measure that this improved performance? Could you please post the numbers?
Laszlo Vidacs
Comment 3
2014-01-07 04:04:40 PST
No measurements performed, just omit some unnecessary method calls. I see a possible problem here only when the length is changed within the loop.
Laszlo Vidacs
Comment 4
2014-02-12 01:42:30 PST
Created
attachment 223955
[details]
Patch
Laszlo Vidacs
Comment 5
2014-02-12 01:43:25 PST
I ran several DOM performance tests on efl, but from this data I cant draw conclusions about the performance of the patch. In fact, at 1 place of the 4 places updated in this patch, the length() method call is meantime moved outside the loop:
http://trac.webkit.org/changeset/162394/trunk/Source/WebCore/dom/ElementData.cpp
So the patch is updated to cope with the remaining three places.
Brent Fulgham
Comment 6
2014-02-19 12:20:30 PST
Comment on
attachment 223955
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=223955&action=review
> Source/WebCore/dom/ElementData.cpp:146 > m_attributeVector.uncheckedAppend(other.m_attributeArray[i]);
Honestly this would probably be better as a new C++11 loop.
Laszlo Vidacs
Comment 7
2014-02-24 06:10:57 PST
Created
attachment 225058
[details]
Patch
Laszlo Vidacs
Comment 8
2014-02-24 06:12:43 PST
Thanks for the feedback, I tried the following code: for (const Attribute& i : other.m_attributeArray) m_attributeVector.uncheckedAppend(i); Unfortunately m_attributeArray is defined as a zero size array (ShareableElementData.h: Attribute m_attributeArray[0];), so the range based loop does not iterate through it. There is a minor change in the patch to use the stored value in reserveCapacity().
WebKit Commit Bot
Comment 9
2014-03-03 02:12:17 PST
Comment on
attachment 225058
[details]
Patch Clearing flags on attachment: 225058 Committed
r164981
: <
http://trac.webkit.org/changeset/164981
>
WebKit Commit Bot
Comment 10
2014-03-03 02:12:22 PST
All reviewed patches have been landed. Closing bug.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug