[Webkit-unassigned] [Bug 27762] [v8] Cache atomic strings in externalized v8 strings

[bugzilla-daemon at webkit.org]

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





--- Comment #38 from David Levin <levin at chromium.org>  2009-07-31 16:32:23 PDT ---
I tried to be brief as possible in this, but it still was long. Sorry.

My overall conclusion:
  If devs want to increase the perf of code using v8 bindings by taking
advantage of a v8 feature and there doesn't seem a clear path to do something
more generic, that seems like a really reasonable thing to do and not require
an equivalent change in JSC. (The reverse goes without saying.)

Details:
1. The big issue seems to be "Why is this needed in V8 and not in something
done for both JSC and V8?"

  For V8, this patch solution uses v8::string::MakeExternal (which stores the
WebCore String and uses that buffer pointer inside of V8).

  JSC uses features of UString to speed up this code (-- for example, keeping
the precomputed hash).  In addition, I believe that JSC could do even by taking
advantage of the fact that it can share the UChar* buffer from the UString.

  After debugging both JSC and V8 on the benchmar, I saw no significant
difference in way that AtomicStrings are formed from UStrings vs String, but
the code path for UString was obviously invested in to make sure it preformed
well.

  I'm not sure why this seemed to be a good optimization for Chromium (and has
not been for Safari), but if there isn't something apparent in AtomicString
that can be fixed, then it seems reasonable to use a feature of v8::String to
handle it.

2. The second issue was "It seems silly to hold two StringImpl pointers, one in
a String and one in an AtomicString."

  A. The WebCoreStringResource may be created with a WebCore::String to begin
with.  (It doesn't make sense to incur the cost of an AtomicString if it isn't
need.)

  B. When WebCoreStringResource is created, it calls MakeExternal with that
WebCore::String.
  At this point the buffer in that WebCore::String must stay around for as long
as the v8::String is alive.  

  C. Later, that same string value may come from v8 and want to be
AtomicString.  In that case, the AtomicString is cached as well due to
performance reasons.

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