[v8] Introduce single element caches for WebCore::String to v8::String conversions
Created attachment 58396 [details] First version
Comment on attachment 58396 [details] First version I removed a check for WTF::MainThread() from getCache function. The idea is that in that case caching should be disabled and thus this function would always return an empty map. If you think this reasoning is too subtle, I'd be glad to revert this modification.
Comment on attachment 58396 [details] First version Cool. WebCore/bindings/v8/V8Binding.cpp: + ASSERT(WTF::isMainThread()); I didn't quite understand the reasoning behind remove this assert. I'm not necessarily against it, I just don't understand. WebCore/bindings/v8/V8Binding.h:84 + // Note: RefPtr is a must as we cache by StringImpl* equality, not identity I don't quite understand the difference between equality and identity, but it makes sense to me why we need a RefPtr here.
Thanks a lot, Adam. Difference between equality and identity: by identity I mean just an address of StringImpl, therefore there might be not identical, but equal string impls (if their contents is the same). If you could suggest better words, that'd be most appreciated. Regarding MainThread. I just speculative fetch the map in any thread. But I don't add anything into it if cache is not allowed (that is we're always on MainThread). Let me roll it back as it is too complicated.
Created attachment 58488 [details] Restore isMainThread assertion
Comment on attachment 58488 [details] Restore isMainThread assertion > Regarding MainThread. I just speculative fetch the map in any thread. I see. I'd have to looking into it more carefully to see whether that's safe. If it doesn't have a big perf impact, it might be netter to leave the assert in so the code is easier to understand.
(In reply to comment #6) > (From update of attachment 58488 [details]) > > Regarding MainThread. I just speculative fetch the map in any thread. > > I see. I'd have to looking into it more carefully to see whether that's safe. If it doesn't have a big perf impact, it might be netter to leave the assert in so the code is easier to understand. Agree. It shouldn't be big perf impact as it is a slow case anyway. Plus condition is perfectly predictable. Once again, thanks a lot for review, cq+'ing.
Comment on attachment 58488 [details] Restore isMainThread assertion Clearing flags on attachment: 58488 Committed r61031: <http://trac.webkit.org/changeset/61031>
All reviewed patches have been landed. Closing bug.
Created attachment 58785 [details] Clean single element cache on major GCs which may invalidate handles
Comment on attachment 58785 [details] Clean single element cache on major GCs which may invalidate handles Ok.
As it got reverted
Comment on attachment 58785 [details] Clean single element cache on major GCs which may invalidate handles Clearing flags on attachment: 58785 Committed r61252: <http://trac.webkit.org/changeset/61252>