[Webkit-unassigned] [Bug 31415] [v8] do not copy data twice when converting short v8 string into WebCore::String

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=31415





--- Comment #9 from anton muhin <antonm at chromium.org>  2009-11-18 05:32:37 PST ---
(In reply to comment #8)
> > It has no performance impact or completely minimal.  The idea was not to speed
> > up some things, but to make it clear that this optimization is actually wrong
> > for this particular case.
> 
> I'm not sure I understand the idea of the patch then. It replaces 15 lines of
> code with 43, to document that certain optimization does not always produce a
> perf win... Could a comment suffice? 

It is an optimization (for small strings we do not copy them twice).  But perf
effect as measured by current benchmarks is minimal if any.  However, I think
it should be easy to cook up a benchmark that shows the benefit---many
conversions of short strings.

I am not sure number of lines removed/added is a good estimator.  E.g. I
removed two lines and added 10 to use a common idiom of traits when fetching
webcore string from the resource (instead of overloading which is less
intuitive imho).

> It seems the purpose of v8StringToWebCoreString templated function is to fold
> implementation of 2 other functions, v8StringToWebCoreString and
> v8StringToAtomicWebCoreString, into one, to avoid code duplication. It is
> strange to branch into 2 separate implementations again from
> v8StringToWebCoreString template. Perhaps 2 separate implementations from the
> start would be more readable?

The problem is there is no trivial prologue (checking if the string is already
externalized) and epilogue (which externalizes the string if necessary) which
I'd like not to duplicate.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.