[webkit-reviews] review denied: [Bug 70698] Removed some String misfeatures : [Attachment 112118] Patch

[bugzilla-daemon at webkit.org]

David Levin <levin at chromium.org> has denied Geoffrey Garen <ggaren at apple.com>'s
request for review:
Bug 70698: Removed some String misfeatures
https://bugs.webkit.org/show_bug.cgi?id=70698

Attachment 112118: Patch
https://bugs.webkit.org/attachment.cgi?id=112118&action=review

------- Additional Comments from David Levin <levin at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=112118&action=review


> Source/JavaScriptCore/ChangeLog:17
> +	   a threading solution by pure inertia.)

Actually this is incorrect on several levels:

1. There is evidence of a speed up: http://trac.webkit.org/changeset/44325 for
sharing across JSC and WebCore but maybe you meant across threads. I don't
remember if I did that measurement. It was meant for the xml case when the
buffer is shared across threads and that buffer may be huge.

2. It shouldn't be happening for small strings (unless someone broke that), but
it was specifically tuned to pick a sweet spot for the length at which to copy.


3. The whole point of this feature was due to workers. I had to make UString
(ScriptString) shareable across threads. I can't remember why I added the other
feature to share string between WebCore and JavaScriptCore but that was a side
thing.

Frankly there is something that is slightly broken in this feature due to what
someone else did, but it is fixable -- however removing it will not fix things.


So in short please don't do this.