Skip to content

Conversation

@aardgoose
Copy link
Contributor

Workaround for #13069 issues

@mrdoob
Copy link
Owner

mrdoob commented Jan 11, 2018

var _Math = {

	generateUUID: ( function () {

		// http://stackoverflow.com/questions/105034/how-to-create-a-guid-uuid-in-javascript/21963136#21963136

		var lut = [];

		for ( var i = 0; i < 256; i ++ ) {

			lut[ i ] = ( i < 16 ? '0' : '' ) + ( i ).toString( 16 ).toUpperCase();

		}

		return function generateUUID() {

			var d0 = Math.random() * 0xffffffff | 0;
			var d1 = Math.random() * 0xffffffff | 0;
			var d2 = Math.random() * 0xffffffff | 0;
			var d3 = Math.random() * 0xffffffff | 0;
			return lut[ d0 & 0xff ] + lut[ d0 >> 8 & 0xff ] + lut[ d0 >> 16 & 0xff ] + lut[ d0 >> 24 & 0xff ] + '-' +
				lut[ d1 & 0xff ] + lut[ d1 >> 8 & 0xff ] + '-' + lut[ d1 >> 16 & 0x0f | 0x40 ] + lut[ d1 >> 24 & 0xff ] + '-' +
				lut[ d2 & 0x3f | 0x80 ] + lut[ d2 >> 8 & 0xff ] + '-' + lut[ d2 >> 16 & 0xff ] + lut[ d2 >> 24 & 0xff ] +
				lut[ d3 & 0xff ] + lut[ d3 >> 8 & 0xff ] + lut[ d3 >> 16 & 0xff ] + lut[ d3 >> 24 & 0xff ];

		};

	} )(),

	generateUUID2: ( function () {

		// http://stackoverflow.com/questions/105034/how-to-create-a-guid-uuid-in-javascript/21963136#21963136

		var lut = [];

		for ( var i = 0; i < 256; i ++ ) {

			lut[ i ] = ( i < 16 ? '0' : '' ) + ( i ).toString( 16 );

		}

		return function generateUUID() {

			var d0 = Math.random() * 0xffffffff | 0;
			var d1 = Math.random() * 0xffffffff | 0;
			var d2 = Math.random() * 0xffffffff | 0;
			var d3 = Math.random() * 0xffffffff | 0;

			return String( lut[ d0 & 0xff ] + lut[ d0 >> 8 & 0xff ] + lut[ d0 >> 16 & 0xff ] + lut[ d0 >> 24 & 0xff ] + '-' +
				lut[ d1 & 0xff ] + lut[ d1 >> 8 & 0xff ] + '-' + lut[ d1 >> 16 & 0x0f | 0x40 ] + lut[ d1 >> 24 & 0xff ] + '-' +
				lut[ d2 & 0x3f | 0x80 ] + lut[ d2 >> 8 & 0xff ] + '-' + lut[ d2 >> 16 & 0xff ] + lut[ d2 >> 24 & 0xff ] +
				lut[ d3 & 0xff ] + lut[ d3 >> 8 & 0xff ] + lut[ d3 >> 16 & 0xff ] + lut[ d3 >> 24 & 0xff ] ).toUpperCase();

		};

	} )()

};

function run( func ) {

	var loop = 0x100000;

	var startTime = performance.now();

	for ( var i = 0; i < loop; i ++ ) {

		func();

	}

	var endTime = performance.now();

	console.log( ( endTime - startTime ).toFixed( 2 ) + 'ms, loop=' + loop );

}

run(_Math.generateUUID);
run(_Math.generateUUID);
run(_Math.generateUUID2);
run(_Math.generateUUID2);

@mrdoob
Copy link
Owner

mrdoob commented Jan 11, 2018

Goes from ~350ms to ~740ms here... 😕

@makc
Copy link
Contributor

makc commented Jan 11, 2018

why does 3js use uuids at all? can't it just use counters?

@mrdoob
Copy link
Owner

mrdoob commented Jan 11, 2018

why does 3js use uuids at all? can't it just use counters?

Mainly for serialising/deserialising.

@makc
Copy link
Contributor

makc commented Jan 11, 2018

deserialising

ok, and why can't you do geometries[123] instead of geometries['XXX-YYY-SOMETHING'] while deserialising?

@mrdoob
Copy link
Owner

mrdoob commented Jan 11, 2018

I'm wondering if uppercasing the uuid is worth it though. As far as I understand, uppercase uuid consumes 10x more memory than lowercase?

@mrdoob
Copy link
Owner

mrdoob commented Jan 11, 2018

ok, and why can't you do geometries[123] instead of geometries['XXX-YYY-SOMETHING'] while deserialising?

Lets not derail this thread.

@makc
Copy link
Contributor

makc commented Jan 11, 2018

in the context of #13069 .toLowerCase() would probably have the same effect

@aardgoose
Copy link
Contributor Author

the performance hit is why i looked at removing the requirement for the uuid's with the lazy_uuid PR.

The WeakMap.get( object ) is probably a simpler lookup just on the object itself rather than having to get an object property as the uuid lookup in WebGLProperties & Attributes so that is probably a gain too.

@mrdoob the case of the uuid doesn't matter. It just gets the VM to create a more compact copy as a side effect - hence the hacky nature.

@mrdoob
Copy link
Owner

mrdoob commented Jan 11, 2018

How about keeping the toUpperCase() for the LUT and doing a String() in the return?

( function () {

	// http://stackoverflow.com/questions/105034/how-to-create-a-guid-uuid-in-javascript/21963136#21963136

	var lut = [];

	for ( var i = 0; i < 256; i ++ ) {

		lut[ i ] = ( i < 16 ? '0' : '' ) + ( i ).toString( 16 ).toUpperCase();

	}

	return function generateUUID() {

		var d0 = Math.random() * 0xffffffff | 0;
		var d1 = Math.random() * 0xffffffff | 0;
		var d2 = Math.random() * 0xffffffff | 0;
		var d3 = Math.random() * 0xffffffff | 0;

		return String( lut[ d0 & 0xff ] + lut[ d0 >> 8 & 0xff ] + lut[ d0 >> 16 & 0xff ] + lut[ d0 >> 24 & 0xff ] + '-' +
			lut[ d1 & 0xff ] + lut[ d1 >> 8 & 0xff ] + '-' + lut[ d1 >> 16 & 0x0f | 0x40 ] + lut[ d1 >> 24 & 0xff ] + '-' +
			lut[ d2 & 0x3f | 0x80 ] + lut[ d2 >> 8 & 0xff ] + '-' + lut[ d2 >> 16 & 0xff ] + lut[ d2 >> 24 & 0xff ] +
			lut[ d3 & 0xff ] + lut[ d3 >> 8 & 0xff ] + lut[ d3 >> 16 & 0xff ] + lut[ d3 >> 24 & 0xff ] );

	};

} )()

No performance changes with that code here. Does this solve the memory issue?

@mrdoob
Copy link
Owner

mrdoob commented Jan 11, 2018

For testing...

var _Math = {

	generateUUID: ( function () {

		// http://stackoverflow.com/questions/105034/how-to-create-a-guid-uuid-in-javascript/21963136#21963136

		var lut = [];

		for ( var i = 0; i < 256; i ++ ) {

			lut[ i ] = ( i < 16 ? '0' : '' ) + ( i ).toString( 16 ).toUpperCase();

		}

		return function generateUUID() {

			var d0 = Math.random() * 0xffffffff | 0;
			var d1 = Math.random() * 0xffffffff | 0;
			var d2 = Math.random() * 0xffffffff | 0;
			var d3 = Math.random() * 0xffffffff | 0;
			return lut[ d0 & 0xff ] + lut[ d0 >> 8 & 0xff ] + lut[ d0 >> 16 & 0xff ] + lut[ d0 >> 24 & 0xff ] + '-' +
				lut[ d1 & 0xff ] + lut[ d1 >> 8 & 0xff ] + '-' + lut[ d1 >> 16 & 0x0f | 0x40 ] + lut[ d1 >> 24 & 0xff ] + '-' +
				lut[ d2 & 0x3f | 0x80 ] + lut[ d2 >> 8 & 0xff ] + '-' + lut[ d2 >> 16 & 0xff ] + lut[ d2 >> 24 & 0xff ] +
				lut[ d3 & 0xff ] + lut[ d3 >> 8 & 0xff ] + lut[ d3 >> 16 & 0xff ] + lut[ d3 >> 24 & 0xff ];

		};

	} )(),

	generateUUID2: ( function () {

		// http://stackoverflow.com/questions/105034/how-to-create-a-guid-uuid-in-javascript/21963136#21963136

		var lut = [];

		for ( var i = 0; i < 256; i ++ ) {

			lut[ i ] = ( i < 16 ? '0' : '' ) + ( i ).toString( 16 ).toUpperCase();

		}

		return function generateUUID() {

			var d0 = Math.random() * 0xffffffff | 0;
			var d1 = Math.random() * 0xffffffff | 0;
			var d2 = Math.random() * 0xffffffff | 0;
			var d3 = Math.random() * 0xffffffff | 0;

			return String(
				lut[ d0 & 0xff ] + lut[ d0 >> 8 & 0xff ] + lut[ d0 >> 16 & 0xff ] + lut[ d0 >> 24 & 0xff ] + '-' +
				lut[ d1 & 0xff ] + lut[ d1 >> 8 & 0xff ] + '-' + lut[ d1 >> 16 & 0x0f | 0x40 ] + lut[ d1 >> 24 & 0xff ] + '-' +
				lut[ d2 & 0x3f | 0x80 ] + lut[ d2 >> 8 & 0xff ] + '-' + lut[ d2 >> 16 & 0xff ] + lut[ d2 >> 24 & 0xff ] +
				lut[ d3 & 0xff ] + lut[ d3 >> 8 & 0xff ] + lut[ d3 >> 16 & 0xff ] + lut[ d3 >> 24 & 0xff ]
			);

		};

	} )()

};

function run( func ) {

	var loop = 0x100000;

	var startTime = performance.now();

	for ( var i = 0; i < loop; i ++ ) {

		func();

	}

	var endTime = performance.now();

	console.log( ( endTime - startTime ).toFixed( 2 ) + 'ms, loop=' + loop );

}

run(_Math.generateUUID);
run(_Math.generateUUID);
run(_Math.generateUUID2);
run(_Math.generateUUID2);

@aardgoose
Copy link
Contributor Author

aardgoose commented Jan 11, 2018

Do a heap dump in Chrome and look at the (concatenated string)
uuid
section. You can see the way it stores the strings in lots of small sections.

I think just String() alone just produces a reference to the bloated version hence no performance change and no reduction of memory.

Just checked - just String() still leaves the strings in the fragmented form. I thought I had tried that myself.

@makc
Copy link
Contributor

makc commented Jan 11, 2018

out of interest just tried to do this with Uint8Array and TextDecoder, and it's only 15 times slower :D then I thought if I inline setHex, I can gain something, but profiler says -23% max
screen shot 2018-01-11 at 23 05 30

compared to this, toUpperCase x2 perfrmance drop does not look that bad at all.

@takahirox
Copy link
Collaborator

takahirox commented Jan 12, 2018

BTW, Array.join( '' ) (array includes pointers to one character in another array) used in older generateUUID() seems to allocate one consecutive memory space. You can try #13069 (comment) by replacing r89 with r87. (Seems like this hack with new generateUUID() is still faster than the older one tho. #12432 (comment))

So, this uuid memory consumption issue is new to Three.js(>=r88).

@mcollina
Copy link

If UUIDs are not needed, I would recommend using an approach similar to https://github.com/mcollina/hyperid/blob/master/hyperid.js: generate something unique and then concatenate with a counter.

Also https://www.npmjs.com/package/flatstr.

@takahirox
Copy link
Collaborator

takahirox commented Jan 12, 2018

Number tech is interesting...
But sometimes it doesn't work well so we need to carefully use it?

I tried four patterns with the following code on my Windows10 + Chrome.

<script src="https://rawgit.com/mrdoob/three.js/r89/build/three.js"></script>
<script>
var num = 0x10000;

function flatstr( s ) {

    Number( s );
    return s;

}

var array = Array( num );

for ( var i = 0; i < num; i ++ ) {

    // 1.
    //array[ i ] = THREE.Math.generateUUID();

    // 2.
    //array[ i ] = THREE.Math.generateUUID().toUpperCase();

    // 3.
    //array[ i ] = flatstr( THREE.Math.generateUUID() );

    // 4.
    //var uuid = THREE.Math.generateUUID();
    //Number( uuid );
    //array[ i ] = uuid;

}
</script>
concatenated string count shallow size retained size total heap snapshot size
1. 921,259 36,850,360 39,507,080 42.3MB
2. 3,755 150,200 185,480 8.8MB
3. 809,262 32,370,480 34,776,728 38.2MB
4. 52,893 2,115,720 5,295,832 10.7MB

I expected 3.'s and 4.'s result are same as the 2.'s result but seems like a lot of concatenated strings are left in heap.

@aardgoose
Copy link
Contributor Author

I was half expecting this to be a recent issue, since I hadn't seen the problem before when looking at heap size in general.

I would check what you get out from Number( uuid ). console.log( Number ( new THREE.Object3D().uuid ) ); produces NaN.

@takahirox
Copy link
Collaborator

Probably what Number( uuid ) produces doesn't matter in this tech if I'm right.

@makc
Copy link
Contributor

makc commented Jan 13, 2018

@takahirox and from 42 vs 38 mb it does not seem it produces anything at all

@makc makc mentioned this pull request Jan 29, 2018
11 tasks
@aardgoose aardgoose closed this Mar 1, 2018
@aardgoose aardgoose deleted the uuid-flatten branch March 1, 2018 14:43
@takahirox
Copy link
Collaborator

3 works now by replacing flatstr with

function flatstr( s ) {

    return isNaN( Number( s ) ) ? s :  '';

}

Probably we need to let Number( s ) not be removed by JS engine optimization.

Bad news, Number( s ) tech isn't fast. A bit slower than .toUpperCase(). I speculate String::Flatten method itself isn't fast regardless how we trigger.

@takahirox
Copy link
Collaborator

Would you reopen this because of #13069 (comment)?

var d3 = Math.random() * 0xffffffff | 0;
return lut[ d0 & 0xff ] + lut[ d0 >> 8 & 0xff ] + lut[ d0 >> 16 & 0xff ] + lut[ d0 >> 24 & 0xff ] + '-' +

return String( lut[ d0 & 0xff ] + lut[ d0 >> 8 & 0xff ] + lut[ d0 >> 16 & 0xff ] + lut[ d0 >> 24 & 0xff ] + '-' +
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I prefer removing "String", not strong preference tho.

@mrdoob
Copy link
Owner

mrdoob commented Mar 14, 2018

I can't reopen this PR because @aardgoose deleted the branch.

@takahirox
Copy link
Collaborator

Oops, then I make a new one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants