Bug 39892 – Make NodeList getters take AtomicString instead of plain String

• Status: RESOLVED
• Resolution: FIXED
• Component: DOM

[anton muhin]

Make NodeList getters take AtomicString instead of plain String

[anton muhin]

Created attachment 57355 [details]
Patch

[anton muhin]

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?)

[Darin Adler]

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.

[Darin Adler]

(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.

[anton muhin]

Created attachment 57356 [details]
Patch

[anton muhin]

(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+.

[anton muhin]

Created attachment 57357 [details]
with correct reviewed by

[WebKit Commit Bot]

Comment on attachment 57357 [details]
with correct reviewed by

Clearing flags on attachment: 57357

Committed r60405: <http://trac.webkit.org/changeset/60405>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Eric Seidel (no email)]

Reopening since this was rolled out.

[Darin Adler]

How did this break the build?

[Darin Adler]

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.

[Xan Lopez]

(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.

[anton muhin]

(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.

[WebKit Commit Bot]

Comment on attachment 57357 [details]
with correct reviewed by

Clearing flags on attachment: 57357

Committed r60434: <http://trac.webkit.org/changeset/60434>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Lucas Forschler]

Mass moving XML DOM bugs to the "DOM" Component.