Some rather ad hoc testing shows that if bytearray accesses hit the bytearray check first, image data access time would be improved by ~10-15%
Created attachment 26605 [details] Specialise get/put_by_val Nice and simple
Comment on attachment 26605 [details] Specialise get/put_by_val r=me
Committing to http://svn.webkit.org/repository/webkit/trunk ... M JavaScriptCore/ChangeLog M JavaScriptCore/interpreter/Interpreter.cpp M JavaScriptCore/interpreter/Interpreter.h Committed r39796
Comment on attachment 26605 [details] Specialise get/put_by_val I know this was already reviewed, but I have a few comments. > + JSValuePtr result; > + unsigned i; > + > + bool isUInt32 = JSImmediate::getUInt32(subscript, i); I usually like to group the declaration of an out argument like "i" with the function call it's coming out of, rather than having it separated by a blank line. > + if (LIKELY(isUInt32)) { > + if (interpreter->isJSByteArray(baseValue) && asByteArray(baseValue)->canAccessIndex(i)) > + return JSValuePtr::encode(asByteArray(baseValue)->getIndex(i)); You need a comment explaining why this case does a return when all the other cases use "result" instead. I have no doubt there's a good reason, perhaps to save code since there can't be an exception in this case, but withut a comment it's a subtle point many people might missing. > double dValue = 0; > + ctiPatchCallByReturnAddress(STUB_RETURN_ADDRESS, reinterpret_cast<void*>(cti_op_put_by_val_byte_array)); > if (JSImmediate::isNumber(value)) { > jsByteArray->setIndex(i, JSImmediate::getTruncatedInt32(value)); > return; It seems a real shame that this code defines and initialized a double that's not used in the isNumber fast case. Can you move the definition of dValue down a bit? > + double dValue = 0; > + if (JSImmediate::isNumber(value)) { > + jsByteArray->setIndex(i, JSImmediate::getTruncatedInt32(value)); > + return; > + } else if (fastIsNumber(value, dValue)) { > + jsByteArray->setIndex(i, dValue); > + return; > + } else > + baseValue->put(callFrame, i, value); We normally don't do else after return. And again, I think those "return" need brief comments.