Closed GoogleCodeExporter closed 9 years ago
I can't implement uniqueness in my hash function, because ANGLE (obviously!)
expects the same result from identical input. But it shouldn't really need to
call the hash more than once per name.
Also, maybe the hashing should be internal to ANGLE? Why allow implementations
to set a function? It should just be a flag to mangle names or not.
Original comment by d...@grorg.org
on 24 Sep 2013 at 12:07
The motivation to use a hash function came from Mozilla. Varying variables and
uniforms share a namespace across shaders, so it's required to produce the same
mangled name globally, not just per-shader. Mozilla pointed out that doing this
requires maintaining an ever-growing table of names, effectively causing a
memory leak as WebGL programs start up and stop. The solution decided upon was
to use a hash function.
Original comment by kbr@chromium.org
on 24 Sep 2013 at 12:25
To answer your question "hashing should be internal to ANGLE?", we decided
(after discussion with firefox) that we should allow browsers to decide their
own hashing functions. Chromium and Firefox already have hashing function in
their code base, so no need to link another one unnecessarily.
Chromium uses CityHash (https://code.google.com/p/cityhash/).
Original comment by z...@google.com
on 24 Sep 2013 at 12:33
Sure - I get that we need constant name mangling across programs (sorry for
mentioning shader-unique numbers), it's just that the consequences of a clash
are pretty severe. And there are well known clashes in CityHash.
On the other hand, the number of programs/shaders a particular context will
compile in its lifetime probably isn't that huge. And since you already have to
keep the table of original symbol -> mangled symbol mappings (for binding),
it's not as if you're saving much memory. It also shows a little weirdness in
the API: I provide a hash function but don't control how it is used, and then
later I have to copy out the list of symbol mappings.
I think in WebKit I'll do a mixture of hash (we only have a 32 bit hash
available) + some context-wide incrementing number. There will be a memory
increase, but maybe I can share the shader symbol table (which I actually can't
do, since I don't technically know what ANGLE does with the hash)
I still think it would be a much nicer API if I simply asked ANGLE to mangle
all names and never had to worry about hashing. As I said, I already have to
record the original -> mangled symbol table.
Original comment by d...@grorg.org
on 24 Sep 2013 at 4:14
> Mozilla pointed out that doing this requires maintaining an ever-growing
table of names, effectively causing a memory leak as WebGL programs start up
and stop.
They have to keep the table of names around anyway as long as the program is
active. Only when a program (and shader) is deleted can you free the symbol
mapping. The current API makes it awkward to manage that. If you truly want to
make sure you never clash names, then you need to search the entire active
global name space (except ANGLE can't do that, because it doesn't have access).
Original comment by d...@grorg.org
on 24 Sep 2013 at 4:20
The freeing of symbol mapping has nothing to do ANGLE. ANGLE side mapping is
lost at the next conpile, and Browser side is responsible to
query/cache/release this mapping on its own.
I think the whole purpose of cashing is we don't have to maintain a global name
space, with the assumption that hashing clash possibility is so tiny that we
can ignore them. So far we haven't encountered any real life examples of
hashing clash. We could evaluate various hashing functions and select the best
one (perf wise and clash wise). That's one of the reasons we didn't select a
hashing in ANGLE but let each browser makes their own decisions.
Original comment by z...@google.com
on 24 Sep 2013 at 11:55
Closing as not-an-actual-issue-report, as this was more of a discussion and
seems to have concluded.
Original comment by shannonw...@chromium.org
on 15 Nov 2013 at 2:10
Original issue reported on code.google.com by
d...@grorg.org
on 24 Sep 2013 at 12:04