Simplify JSFunction creation for functions written in JS
Created attachment 102447 [details] Patch
Comment on attachment 102447 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=102447&action=review > Source/JavaScriptCore/runtime/Executable.cpp:147 > + m_jsName.set(globalData, name.isEmpty() ? globalData.smallStrings.emptyString(&globalData) : jsString(&globalData, name.ustring())); Can this be done with construction instead of assignment? Would that be a tiny bit better for performance? This expression is long enough that it might be nice to share this code with an inline function instead of repeating it twice. The easiest way to do that would be an inline member function, or we could use an inline non-member function if we are doing it in construction. Doesn’t jsString already handle the empty string case? Why do we need a special case for it here? > Source/JavaScriptCore/runtime/Executable.h:481 > + JSString* jsName() const { return m_jsName.get(); } I would think of this as nameValue or wrappedName rather than jsName.
Comment on attachment 102447 [details] Patch r- because i just copied the old logic (which did an empty string check) and i don't want to just blindly commit. That said, we do have to do the setting of m_jsName late as jsString() may perform a gc allocation, and it is _never_ safe to do GC allocation in the initializer list (you may end up doing a gc sweep midway through executing your initializer list, and then you end up not marking yourself correctly :-( )
Created attachment 102717 [details] Patch
Comment on attachment 102717 [details] Patch You may want to rename jsName to nameValue per Darin's suggestion, and functionNameOffset() should probably return a size_t. Otherwise all looks good.
Created attachment 102720 [details] Patch
Committed r92250: <http://trac.webkit.org/changeset/92250>