[Webkit-unassigned] [Bug 62653] [V8][Chromium] Make StringCache in V8 bindings per-isolate

[bugzilla-daemon at webkit.org]

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





--- Comment #16 from Dmitry Lomov <dslomov at google.com>  2011-06-16 08:06:01 PST ---
(In reply to comment #15)
> Overall, I still have concerns about approach taken.
>   
> And most probably slower---even if each separate patch makes as slower by ~1% which is indistinguishable from noise, we may die of death of million cuts.

I am cautiously optimistic on perf here. Since all we introduce here are TLS lookups for isolates, if we see a lot of those on hot paths, we hopefully be able to fetch those from v8::Object and other V8 entities, as I suggested in the other e-mail. The current perf results do not warrant that though - and this patch is Strings - by far our hottest.

> 
> I would really prefer if there was a separate branch with the full implementation and if overall complexity and perf is good enough, we'll start to bring the changes to WebCore.

I have been thinking about this, and my feeling is that the changes I make here or in other patches are not complex enough to warrant another branch - what do people think? 

I have made a preliminary "spear-headed" patch for isolates as well:
https://bug-61016-attachments.webkit.org/attachment.cgi?id=95694
(I CC'ed you on 61016 as well). It does not hoist much into V8BindingPerIsolateData, but it disables StringCache, which has bad perf implications.

> 
> I would also appreciate more thorough discussion of different approaches, their benefits and drawbacks.  For example, Vitaly (p.c.) noted that we probably do not need to make all FunctionTenplate into per-isolate data as the set of ones used in Workers vs. ones in DOM pages has minimal (if any) intersection.

So what is the concern with FunctionTemplates?
If it is about perf, I didn't observe any change. FunctionTemplates are lazily created, so only the those that are actually used in the isolate will be created.
If it is about the design, I think what has been done is the most simple and straightforward thing - I actually do not see any viable alternatives - but maybe I am missing something?

Regarding StringCache, I actually do not see any viable solution other that what has been done here, or disabling it entirely. What do you think?

> Moving all this stuff makes things more complex.

Well, "complexity" is hard to define precisely.

I think we are on a very straightforward trajectory here for supporting multi-threaded access to multiple instances of V8 in WebCore.  V8BindingPerIsolateData is a natural extension of V8::Isolate for binding-specific data. Hoisting things out of statics is a very natural thing we have to do to support multi-threading. 

If we look from the point of view of multithreading, static data and globals scattered all over the code _is_ complexity, and hoisting it all up in a single state object is _reducing_ complexity.

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