Because even though it doesn't show very highly on profiles, it still is typically the #1 WebGL-related symbol on profiles. Note: this is orthogonal to bug 732660.

Just checked on Banabread level 2: - without patch, ValidateBuffer is the top WebGLContext symbol at 0.75%. perf annotate shows that that time is primarily spent in CheckedInt arithmetic, and secondarily in if()'s that can't be branch-predicted because of erratic outcome (the if (!vd.enabled) and the if (!mCurrentProgram->IsAttribInUse(i))). - with patch, ValidateBuffer is down to 0.07%.

Comment on attachment 659917 [details] [diff] [review] Cache the result of WebGLContext::ValidateBuffers Review of attachment 659917 [details] [diff] [review]: ----------------------------------------------------------------- nits: ::: content/canvas/src/WebGLContext.h @@ +1124,5 @@ > int32_t mGLMaxVaryingVectors; > int32_t mGLMaxFragmentUniformVectors; > int32_t mGLMaxVertexUniformVectors; > > + // Cache the max number of element that can be read from bound VBOs elements @@ +1125,5 @@ > int32_t mGLMaxFragmentUniformVectors; > int32_t mGLMaxVertexUniformVectors; > > + // Cache the max number of element that can be read from bound VBOs > + // (result of ValidateBuffers) for better performances. performance @@ +1126,5 @@ > int32_t mGLMaxVertexUniformVectors; > > + // Cache the max number of element that can be read from bound VBOs > + // (result of ValidateBuffers) for better performances. > + int32_t mCachedMaxReadFromVBO; suggested name: mMinInUseBufferLength ???? @@ +1128,5 @@ > + // Cache the max number of element that can be read from bound VBOs > + // (result of ValidateBuffers) for better performances. > + int32_t mCachedMaxReadFromVBO; > + > + inline void InvalidateCachedMaxReadFromVBO() Same ::: content/canvas/src/WebGLContextGL.cpp @@ +393,5 @@ > void > WebGLContext::BlendFuncSeparate(WebGLenum srcRGB, WebGLenum dstRGB, > WebGLenum srcAlpha, WebGLenum dstAlpha) > { > + InvalidateCachedMaxReadFromVBO(); X @@ +417,5 @@ > GLsizeiptr size, > const GLvoid *data, > GLenum usage) > { > + InvalidateCachedMaxReadFromVBO(); Only do it in BufferData @@ +443,5 @@ > NS_IMETHODIMP > WebGLContext::BufferData(WebGLenum target, const JS::Value& data, GLenum usage, > JSContext* cx) > { > + InvalidateCachedMaxReadFromVBO(); No need to do it in the old-bindings functions.

Comment on attachment 660094 [details] [diff] [review] Cache the result of WebGLContext::ValidateBuffers Review of attachment 660094 [details] [diff] [review]: ----------------------------------------------------------------- Changed my mind --- please change before committing: ::: content/canvas/src/WebGLContext.h @@ +1126,5 @@ > int32_t mGLMaxVertexUniformVectors; > > + // Cache the max number of elements that can be read from bound VBOs > + // (result of ValidateBuffers). > + int32_t mMinInUseBufferLength; Actually, I was wrong: this should be AttribArrayLength, not BufferLength. Indeed this has nothing to do with the length of the buffer (in bytes) and everything to do with how many vertices can be drawn i.e. the attrib array length @@ +1128,5 @@ > + // Cache the max number of elements that can be read from bound VBOs > + // (result of ValidateBuffers). > + int32_t mMinInUseBufferLength; > + > + inline void InvalidateCachedMinInUseBufferLength() Same.

I should have caught that during review: these calls need to be moved down a bit, ideally just before the point where we start modifying any state as a result of the current WebGL function call.

Comment on attachment 660824 [details] [diff] [review] move the InvalidateCachedMinInUseAttribArrayLength calls a bit Review of attachment 660824 [details] [diff] [review]: ----------------------------------------------------------------- Indenting nit, but r+ otherwise. ::: content/canvas/src/WebGLContextGL.cpp @@ +3712,5 @@ > if (!ValidateObject("linkProgram", program)) > return; > > + InvalidateCachedMinInUseAttribArrayLength(); // we do it early in this function > + // as some of the validation below changes program state Bad indenting here.