[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 #6 from anton muhin <antonm at chromium.org>  2009-07-28 06:22:30 PDT ---
David, all, sorry for delay

(In reply to comment #2)
> (From update of attachment 33453 [details])
> Just a few things to clean up.
> 
> > Index: third_party/WebKit/WebCore/bindings/v8/V8Binding.cpp
> 
> > +HashMap<StringImpl*, v8::String*> stringCache;
> 
> Better to use DEFINE_STATE_LOCAL.  (Then it is allocated on demand instead of
> at start up and it won't have to get deleted on shutdown. You could make some
> statically linked function to expose this.)
> 

Done.  I just wanted to keep the code for main thread as fast as possible.

> > +
> > +static void cachedStringCallback(v8::Persistent<v8::Value> wrapper, void* parameter)
> > +{
> 
> Assert that this is only called on the main thread (since HashMap isn't
> threadsafe).

Now not applicable as I have (hopefully) proper support for several threads.

> 
> > +    StringImpl* stringImpl = static_cast<StringImpl*>(parameter);
> > +    ASSERT(stringCache.contains(stringImpl));
> > +    stringCache.remove(stringImpl);
> > +    wrapper.Dispose();
> >  }
> >  
> >  v8::Local<v8::String> v8ExternalString(const String& string)
> 
> A "to" prefix is usually used on conversion functions like this.  (Or perhaps
> you want findCachedV8String?)

David, that's a part of v8 bindings API.  May we postpone it for another CL (if
to touch it at all)?

> 
> >  {
> 
> Assert that this is only called on the main thread (since HashMap isn't
> threadsafe). (As long as, that's true.)

See above.

> 
> > +
> > +    StringImpl* stringImpl = string.impl();
> > +    v8::String* s = stringCache.get(stringImpl);
> > +    if (s) return v8::Local<v8::String>(s);
> 
> Avoid abbreviations:
> s/s/cachedV8String/

Done.

Replying to second batch of comments in the corresponding place.

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