Bug 223757 – Embiggen maximum HashTable size when not using ANGLE

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Template Framework

[Don Olmstead]

A HashMap of sh::ShaderVariable is over the limit introduced in https://trac.webkit.org/changeset/274603/webkit

In file included from ../../Source/WebCore/platform/graphics/opengl/GraphicsContextGLOpenGLCommon.cpp:30:
In file included from ../../Source/WebCore/platform/graphics/opengl/GraphicsContextGLOpenGL.h:30:
In file included from ../../Source/WebCore/platform/graphics\GraphicsContextGL.h:32:
In file included from ../../Source/WebCore/platform/graphics/Image.h:30:
In file included from ../../Source/WebCore/platform/graphics/DecodingOptions.h:28:
In file included from ../../Source/WebCore/platform/graphics\IntSize.h:29:
In file included from WTF/Headers\wtf/JSONValues.h:35:
In file included from WTF/Headers\wtf/HashMap.h:25:
WTF/Headers\wtf/HashTable.h:671:9: error: static_assert failed due to requirement 'sizeof(WTF::String) + sizeof(WTF::KeyValuePair<WTF::String, sh::ShaderVariable>) < 250' "Your HashTable types are too big to efficiently move when rehashing.  Consider using UniqueRef instead"
        static_assert(sizeof(Key) + sizeof(Value) < 250, "Your HashTable types are too big to efficiently move when rehashing.  Consider using UniqueRef instead");
        ^             ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

....
                  ^
../../Source/WebCore/platform/graphics/opengl/GraphicsContextGLOpenGLCommon.cpp:301:63: note: in instantiation of member function 'WTF::HashMap<WTF::String, sh::ShaderVariable, WTF::DefaultHash<WTF::String>, WTF::HashTraits<WTF::String>, WTF::HashTraits<sh::ShaderVariable> >::find' requested here
        const auto& fragmentSymbol = fragmentEntry.varyingMap.find(symbolName);

[Don Olmstead]

Created attachment 424267 [details]
Patch

[Alex Christensen]

Comment on attachment 424267 [details]
Patch

Unfortunately, this is the most straightforward solution.

[EWS]

Committed r275053: <https://commits.webkit.org/r275053>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 424267 [details].

[Radar WebKit Bug Importer]

<rdar://problem/75850257>

[Sam Weinig]

Out of interest, what makes using a UniqueRef, as the assert suggests should be done here, not feasible?

[Alex Christensen]

The maps that are hitting this assertion are our use of ShaderSymbolMap.  The value, sh::ShaderVariable, is too big.  Since sh::ShaderVariable isn't fast allocated, we can't use UniqueRef or makeUnique as they stand.  Our solutions were either

1. Make sh::ShaderVariable fast allocated which would require ANGLE to use WTF
2. Use makeUniqueRefWithoutFastMallocCheck
3. This, which doesn't change existing behavior at all.

Given that ports are moving away from !USE(ANGLE), we decided it would be best to just keep the status quo until that happens.

[Sam Weinig]

(In reply to Alex Christensen from comment #6)
> The maps that are hitting this assertion are our use of ShaderSymbolMap. 
> The value, sh::ShaderVariable, is too big.  Since sh::ShaderVariable isn't
> fast allocated, we can't use UniqueRef or makeUnique as they stand.  Our
> solutions were either
> 
> 1. Make sh::ShaderVariable fast allocated which would require ANGLE to use
> WTF
> 2. Use makeUniqueRefWithoutFastMallocCheck
> 3. This, which doesn't change existing behavior at all.
> 
> Given that ports are moving away from !USE(ANGLE), we decided it would be
> best to just keep the status quo until that happens.

I don't understand why makeUniqueRefWithoutFastMallocCheck wouldn't be the right answer here. Having !USE(ANGLE) in the middle of HashTable seems like more invasive change.

[Alex Christensen]

True.  makeUniqueRefWithoutFastMallocCheck would be better.

[Alex Christensen]

The best solution is to remove all the !USE(ANGLE) code, but this is just in the interim.