RESOLVED WONTFIX 22279
CSSMutableStyleDeclaration can be moved with memcpy
https://bugs.webkit.org/show_bug.cgi?id=22279
Summary CSSMutableStyleDeclaration can be moved with memcpy
George Staikos
Reported 2008-11-15 04:34:26 PST
For efficiency, enable memcpy for copy of CSSMutableStyleDeclaration.
Attachments
patch to enable canMoveWithMemcpy (1.05 KB, patch)
2008-11-15 04:35 PST, George Staikos
no flags
George Staikos
Comment 1 2008-11-15 04:35:25 PST
Created attachment 25185 [details] patch to enable canMoveWithMemcpy
Alexey Proskuryakov
Comment 2 2008-11-16 14:00:05 PST
Could you please give some numbers for the performance improvement you are seeing? The ChangeLog shouldn't say it's safe to copy with memcpy - it is not, but you are only enabling moves. That looks correct to me (I'm not an expert on CSS code though), but I'm worried that this change may make others introduce subtle bugs later.
Yong Li
Comment 3 2008-11-17 09:32:10 PST
This change should be safe as long as the object address is neither directly nor indirectly used in copy constructor. For our profiling data, Vector::resize takes unexpectedly long time for CSSMutableStyleDeclaration and ResourceResponse. We make a conclusion that copy constructor and destructor are called too many time unnecessarily. We're very busy currently. We will get more accurate performance tracking data, and come back to you later when we get time.
Yong Li
Comment 4 2008-11-17 12:20:23 PST
Sorry, I have to correct my last reply. Using memcpy is risky for sure. If any member's address is referenced by another member, do memcpy() upon that object or parent objects can result crash. For example, DeprecatedString, Vector with inline buffer, and any object that contains them. So we would redraw the request.
Yong Li
Comment 5 2008-11-17 12:25:39 PST
(In reply to comment #4) > Sorry, I have to correct my last reply. Using memcpy is risky for sure. If any > member's address is referenced by another member, do memcpy() upon that object > or parent objects can result crash. For example, DeprecatedString, Vector with > inline buffer, and any object that contains them. > > So we would redraw the request. > sorry, I mean withdraw
Geoffrey Garen
Comment 6 2008-11-24 22:16:49 PST
Comment on attachment 25185 [details] patch to enable canMoveWithMemcpy Clearing review flag, based on Yong's comment. Should we close this bug?
George Staikos
Comment 7 2010-04-06 18:05:36 PDT
Decided not to do it.
Note You need to log in before you can comment on or make changes to this bug.