[Webkit-unassigned] [Bug 27655] [v8] cache v8 strings when converting from webcore string to v8 string

[bugzilla-daemon at webkit.org]

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





--- Comment #7 from anton muhin <antonm at chromium.org>  2009-07-28 06:26:48 PDT ---
(In reply to comment #4)
> > I don't know if it's safe to use v8 string from different threads.
> 
> I don't know about v8 strings, but StringImpl is certainly not safe for use on
> multiple threads.

Thanks a lot for explanations.  Looks like I have no option but to have per
thread caches (see the latest patch).  Does it look correct?

> 
> >  If it's the case, I'd only protect the cache with a mutex. 
> 
> That should work for the HashMap (but not the StringImpl in it).

I see, tnx.

> > If it's not, it looks like we need thread locals caches.
> 
> You may want to add to WebCore/platform/ThreadGlobalData.h

I need your advice here.  There is another place in v8 bindings which has
thread local storage---V8DOMMap.  However, they don't keep this stuff into
ThreadGlobalData either (that's why I decided to keep it 'local' as well). 
Overall, I don't see any V8 specific parts in ThreadGlobablData.  Do you think
it's an oversight and should be fixed?

> 
> 
> fyi, I've not had time to look into it, but the function v8ValueToWebCoreString
> in V8Binding.cpp has similar concerns due to its use of AtomicString which may
> not be used across multiple threads.  (I don't know if this can be used from
> workers but if it can be, then it should be fixed as well.)

Thank you very much for spotting this.  Christian is working on this part and
has a CL under review, maybe it'd be more convenient if he fixes it as well.

Thank you very much for review!

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