Bug 39892

Summary: Make NodeList getters take AtomicString instead of plain String
Product: WebKit Reporter: anton muhin <antonm>
Component: DOMAssignee: anton muhin <antonm>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, cdumez, commit-queue, darin, eric, mjs, sam, xan.lopez
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Bug Depends on: 39921    
Bug Blocks:    
Attachments:
Description Flags
Patch
none
Patch
none
with correct reviewed by none

Description anton muhin 2010-05-28 11:40:56 PDT
Make NodeList getters take AtomicString instead of plain String
Comment 1 anton muhin 2010-05-28 11:46:37 PDT
Created attachment 57355 [details]
Patch
Comment 2 anton muhin 2010-05-28 11:47:41 PDT
Those methods turn String into AtomicString later.  And this conversion is faster if underlying string is already atomic.

That buys a small (~2-3%) speed up for Chromium on Dromaeo DOM Core.  I don't know if Safari benefits from it.

(Darin, you beat me at changine platform :)  Is there a way to specify those parameters from command line in webkit-patch?)
Comment 3 Darin Adler 2010-05-28 11:49:47 PDT
Comment on attachment 57355 [details]
Patch

Would be nice if the change log said why this was a good change instead of just stating what the change is.
Comment 4 Darin Adler 2010-05-28 11:50:18 PDT
(In reply to comment #2)
> (Darin, you beat me at changine platform :)  Is there a way to specify those parameters from command line in webkit-patch?)

Not that I know of.
Comment 5 anton muhin 2010-05-28 11:53:50 PDT
Created attachment 57356 [details]
Patch
Comment 6 anton muhin 2010-05-28 11:56:18 PDT
(In reply to comment #4)
> (In reply to comment #2)
> > (Darin, you beat me at changine platform :)  Is there a way to specify those parameters from command line in webkit-patch?)
> 
> Not that I know of.

I see, thanks.

Uploaded a new patch with better message.

I'll try to upload the next one with correct reviewed to start cq+.
Comment 7 anton muhin 2010-05-28 11:57:55 PDT
Created attachment 57357 [details]
with correct reviewed by
Comment 8 WebKit Commit Bot 2010-05-29 04:18:43 PDT
Comment on attachment 57357 [details]
with correct reviewed by

Clearing flags on attachment: 57357

Committed r60405: <http://trac.webkit.org/changeset/60405>
Comment 9 WebKit Commit Bot 2010-05-29 04:18:49 PDT
All reviewed patches have been landed.  Closing bug.
Comment 10 Eric Seidel (no email) 2010-05-29 10:35:21 PDT
Reopening since this was rolled out.
Comment 11 Darin Adler 2010-05-29 20:21:38 PDT
How did this break the build?
Comment 12 Darin Adler 2010-05-29 20:22:58 PDT
Looks to me like the problem with the GTK build was incorrect dependencies in the build system. If that is so, this is going to be an ongoing problem with multiple patches and I don't think rolling out patches is the right way to deal with it.
Comment 13 Xan Lopez 2010-05-30 07:46:09 PDT
(In reply to comment #12)
> Looks to me like the problem with the GTK build was incorrect dependencies in the build system. If that is so, this is going to be an ongoing problem with multiple patches and I don't think rolling out patches is the right way to deal with it.

The right way is to fix the bugs, right. I uploaded a patch to bug 39932 that seems to fix things; once that's in we can try again.
Comment 14 anton muhin 2010-05-31 06:09:50 PDT
(In reply to comment #13)
> (In reply to comment #12)
> > Looks to me like the problem with the GTK build was incorrect dependencies in the build system. If that is so, this is going to be an ongoing problem with multiple patches and I don't think rolling out patches is the right way to deal with it.
> 
> The right way is to fix the bugs, right. I uploaded a patch to bug 39932 that seems to fix things; once that's in we can try again.

Ok, I am trying to cq+ it again.
Comment 15 WebKit Commit Bot 2010-05-31 06:34:58 PDT
Comment on attachment 57357 [details]
with correct reviewed by

Clearing flags on attachment: 57357

Committed r60434: <http://trac.webkit.org/changeset/60434>
Comment 16 WebKit Commit Bot 2010-05-31 06:35:06 PDT
All reviewed patches have been landed.  Closing bug.
Comment 17 Lucas Forschler 2019-02-06 09:03:38 PST
Mass moving XML DOM bugs to the "DOM" Component.