WebGLLights: Inefficient string concatenations

state.hash = state.id + ‘,’ + directionalLength + ‘,’ + pointLength + ‘,’ + spotLength + ‘,’ + rectAreaLength + ‘,’ + hemiLength + ‘,’ + shadows.length;

Related PR: #14568

This line is executed 60 times per second thus produces garbage as seen on Chrome’s Allocation Profiler:

ekran resmi 2018-07-29 12 28 20

The hash value of WebGLLights should be an object instead of concatanated string:

hash: {
stateID: -1,
directionalLength: -1,
pointLength: -1,
spotLength: -1,
rectAreaLength: -1,
hemiLength: -1,
shadowsLength: -1
}

In the WebGLRenderer.js the values should be compared as:

} else if ( materialProperties.lightsHash.stateID !== lights.state.hash.stateID ||
        materialProperties.lightsHash.directionalLength !== lights.state.hash.directionalLength ||
	materialProperties.lightsHash.pointLength !== lights.state.hash.pointLength ||
        materialProperties.lightsHash.spotLength !== lights.state.hash.spotLength ||
	materialProperties.lightsHash.rectAreaLength !== lights.state.hash.rectAreaLength ||
	materialProperties.lightsHash.hemiLength !== lights.state.hash.hemiLength ||
	materialProperties.lightsHash.shadowsLength !== lights.state.hash.shadowsLength ) {

Since JS assigns objects by reference materialProperties.lightsHash should be updated like this to avoid potenial bugs caused by string to object switch:

materialProperties.lightsHash.stateID = lights.state.hash.stateID;
materialProperties.lightsHash.directionalLength = lights.state.hash.directionalLength;
materialProperties.lightsHash.pointLength = lights.state.hash.pointLength;
materialProperties.lightsHash.spotLength = lights.state.hash.spotLength;
materialProperties.lightsHash.rectAreaLength = lights.state.hash.rectAreaLength;
materialProperties.lightsHash.hemiLength = lights.state.hash.hemiLength;
materialProperties.lightsHash.shadowsLength = lights.state.hash.shadowsLength;

instead of:

materialProperties.lightsHash = lights.state.hash

Any ideas/opinions?

Author: Fantashit

1 thought on “WebGLLights: Inefficient string concatenations

  1. @Usnul

    You’re absolutely right abot the hash concept here but this issue is about reducing the GC activity by avoiding creation of new objects in the render loop. In the solution proposed by me the objects are created only once but their properties (which are all integer values, no strings here) are changed inside the loop so it should be quite cheap considering the memory usage. If I understood your proposal correctly
    a new String object would be created inside the render loop anyway when we hash couple of integers together (AFAIK you cannot reuse Strings since they are immutable objects). So your proposal is exactly what I suggest avoiding in this issue.

    I’m not saying it’s not a good idea or anything, it is just against what this issue proposes that’s all 🙂

Comments are closed.