It's quite common for jsStringWithCache() to put tons of duplicate strings in the DOMWrapperWorld's string cache. We could avoid this by making sure that all >1 character strings are atomic strings before caching them.
Created attachment 190491 [details] Possibly a patch
Related to <rdar://problem/12201108>.
Comment on attachment 190491 [details] Possibly a patch I think it would be even better to track these strings back to their sources, and store those sources as AtomicStrings. This has two advantages: (1) Avoids thrashing the AtomicString table when passing strings to JavaScriptCore. (2) Reduces memory use in the DOM. There's nothing special about the JavaScript binding layer that causes likely string duplication -- it's some other layer in the DOM that produced all these duplicates, and hooking in at the JavaScript binding layer is just a convenient work-around.
> There's nothing special about the JavaScript binding layer that causes likely string duplication -- it's some other layer in the DOM that produced all these duplicates, and hooking in at the JavaScript binding layer is just a convenient work-around. I don't think this help for duplicate String, but helps for duplicate JSString. Instead of using AtomicString here, I am in favor of changing JSStringCache to use the String hash instead of the PtrHash. That way we have one cache instead of two. Any opinion?
(In reply to comment #4) > > There's nothing special about the JavaScript binding layer that causes likely string duplication -- it's some other layer in the DOM that produced all these duplicates, and hooking in at the JavaScript binding layer is just a convenient work-around. > > I don't think this help for duplicate String, but helps for duplicate JSString. > > Instead of using AtomicString here, I am in favor of changing JSStringCache to use the String hash instead of the PtrHash. That way we have one cache instead of two. > > Any opinion? I like that approach. FWIW the source of 99% of duplicates on the HTML5 spec are from HTMLAnchorElement::hash() which could be changed to return an AtomicString.
> I don't think this help for duplicate String, but helps for duplicate JSString. We can infer from the duplicate JSStrings that the DOM is wasting memory with duplicate Strings. Solving the duplicate Strings in the DOM will solve both problems in one go. That's why I think it's even better.
(In reply to comment #6) > > I don't think this help for duplicate String, but helps for duplicate JSString. > > We can infer from the duplicate JSStrings that the DOM is wasting memory with duplicate Strings. Solving the duplicate Strings in the DOM will solve both problems in one go. That's why I think it's even better. In this case they are not duplicate Strings in the DOM, but rather the same String generated multiple times every time a getter is called.
> Instead of using AtomicString here, I am in favor of changing JSStringCache to use the String hash instead of the PtrHash. That way we have one cache instead of two. <http://trac.webkit.org/changeset/119341> explains why a String hash was a bad idea at one point. Those criticisms may still apply, or not -- I didn't look deeply.
> <http://trac.webkit.org/changeset/119341> explains why a String hash was a bad idea at one point. So, I re-read this, and the problem with String hash is that it causes the key to be freed if you call HashMap::set(). So, if we switch to String hash, we also need to switch the key to be RefPtr<StringImpl>.
(In reply to comment #9) > > <http://trac.webkit.org/changeset/119341> explains why a String hash was a bad idea at one point. > > So, I re-read this, and the problem with String hash is that it causes the key to be freed if you call HashMap::set(). So, if we switch to String hash, we also need to switch the key to be RefPtr<StringImpl>. I have an idea for something clean, I'll look at that tomorrow. :)