| Summary: | Optimize JSRopeString for resolving directly to AtomicString. | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Andreas Kling <kling> | ||||||
| Component: | JavaScriptCore | Assignee: | Andreas Kling <kling> | ||||||
| Status: | RESOLVED FIXED | ||||||||
| Severity: | Normal | CC: | barraclough, benjamin, cdumez, cgarcia, cmarcelo, commit-queue, darin, esprehn+autocc, fpizlo, ggaren, kangil.han, kling, kondapallykalyan, rniwa | ||||||
| Priority: | P2 | ||||||||
| Version: | 528+ (Nightly build) | ||||||||
| Hardware: | Unspecified | ||||||||
| OS: | Unspecified | ||||||||
| Attachments: |
|
||||||||
|
Description
Andreas Kling
2014-05-04 13:51:48 PDT
Created attachment 230792 [details]
Patch
Created attachment 230793 [details]
Patch
Comment on attachment 230793 [details]
Patch
I would have done this differently. First use the rope in place to find the AtomicString. If none exist, then create the StringImpl and resolve.
Any reason you went for this two steps resolution?
(In reply to comment #3) > (From update of attachment 230793 [details]) > I would have done this differently. First use the rope in place to find the AtomicString. If none exist, then create the StringImpl and resolve. > > Any reason you went for this two steps resolution? For simplicity. Here we walk the fiber tree once instead of up to 3 times. In your version, we'd have to hash recursively, then check equality recursively and then (worst case) copy into a new string recursively. Forgot to mention in ChangeLog, this is a ~20% progression on Dromaeo/dom-query.html Comment on attachment 230793 [details]
Patch
lgtm
Committed r168256: <http://trac.webkit.org/changeset/168256> |