Hi, in #90838, we made TypedArray constructor much faster by casting the argument to a typed array when possible, and using typed array methods instead of going through the javascript path. We can probably introduce the same improvement for the set method (more specifically, in the setWebGLArrayHelper function).
Created attachment 155327 [details] patch proposal with the patch, times for performance test drops from 2600ms to 250ms on my computer
Comment on attachment 155327 [details] patch proposal View in context: https://bugs.webkit.org/attachment.cgi?id=155327&action=review Good performance improvement but needs more cleanup. > Source/WebCore/bindings/js/JSArrayBufferViewHelper.h:63 > + || offset + length < offset) { Please try to figure out a way to avoid duplicating these security related checks. For example, if you pushed the majority of this code into the C++ code instead of doing it in the bindings, you could define an enum return code and conditionally call setDOMException based on that return code. > Source/WebCore/bindings/js/JSArrayBufferViewHelper.h:109 > + break; Refactor this so the code isn't duplicated between here and constructArrayBufferViewWithTypedArrayArgument below. Since this block doesn't refer to any JSC concepts, you could even put it in the C++ code so it could be shared between the JSC and V8 bindings.
Created attachment 155643 [details] updated patch This patch factorize security check and factorize also buffer copy between constructArrayBufferViewWithTypedArrayArgument and setWebGLArrayWithTypedArrayArgument.
Attachment 155643 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'PerformanceTests/Bindings/typed-array-set-..." exit_code: 1 Source/WebCore/bindings/js/JSArrayBufferViewHelper.h:45: Place brace on its own line for function definitions. [whitespace/braces] [4] Source/WebCore/bindings/js/JSArrayBufferViewHelper.h:47: Extra space before ( in function call [whitespace/parens] [4] Source/WebCore/bindings/js/JSArrayBufferViewHelper.h:52: One line control clauses should not use braces. [whitespace/braces] [4] Source/WebCore/bindings/js/JSArrayBufferViewHelper.h:58: One line control clauses should not use braces. [whitespace/braces] [4] Source/WebCore/bindings/js/JSArrayBufferViewHelper.h:120: One line control clauses should not use braces. [whitespace/braces] [4] Total errors found: 5 in 15 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 155645 [details] updated patch Oups. I didn't commit --amend before submitting
Comment on attachment 155645 [details] updated patch View in context: https://bugs.webkit.org/attachment.cgi?id=155645&action=review Thanks, this looks better. One minor naming convention issue I'd appreciate if you'd clean up before committing. r=me > Source/WTF/wtf/TypedArrayBase.h:76 > + bool inbound(unsigned offset, unsigned pos) const Please name this some sort of verb like "checkInboundData".
Created attachment 155922 [details] updated patch
Comment on attachment 155922 [details] updated patch View in context: https://bugs.webkit.org/attachment.cgi?id=155922&action=review You don't need my re-review, so clearing the r? bit. If you fix up the ChangeLog, any WebKit committer can set the cq+ bit on a revised patch. > Source/WTF/ChangeLog:12 > + (WTF::TypedArrayBase::inbound): Rename it here. > Source/WebCore/ChangeLog:3 > + TypedArray set speedup Did you mean to add this?
Created attachment 156125 [details] updated patch
Comment on attachment 156125 [details] updated patch Looks good.
Comment on attachment 156125 [details] updated patch Clearing flags on attachment: 156125 Committed r124483: <http://trac.webkit.org/changeset/124483>
All reviewed patches have been landed. Closing bug.
Moving all JavaScriptGlue bugs to JavaScriptCore. The JavaScriptGlue framework itself is long gone. And most of the more recent bugs put in this component were put there by people who thought this was for some other aspect of “JavaScript glue” and have nothing to do with the actual original reason for the existence of this component, which was an OS-X-only framework named JavaScriptGlue.