-
-
Notifications
You must be signed in to change notification settings - Fork 35.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Object.defineProperty
in THREE.Vector3
constructor causing a significant performance penalty
#21284
Comments
@marcofugaro @DefinitelyMaybe I think we should just do |
I guess that's obvious, but I can see more |
[#176905512] Update to r126 when it's released. See: mrdoob/three.js#21284
I think I'll just make a script and fix all. |
PR: #21285 |
IMHO, you should not be using |
@WestLangley that's a good point, but this is definitely a regression. |
User's have been advised -- certainly by me -- not to call |
FYI, my jstfiddle example is just the simplest test case I could imagine. It doesn't have much in common with the real app. I'm not calling My Thanks for a quick reply and fix! |
@pjanik Right. That is what I assumed. If you are calling The three.js math classes do not use the method at all. Just a friendly tip. :-) |
This is the simpler way. There were reasons for not doing this but I'm happy to try this out and see where we get to. |
Describe the bug
I've noticed a significant performance drop in my application when I upgraded the THREE.JS version. Initially from
0.102.0
to0.125.2
, but I've narrowed it down to a change from0.119.1
to0.120.0
. A single model step in my app (simulation) increased from ~10ms to ~50ms.I've used JS profiler and noticed that Vector3.clone() is at the top now:
I'm using
THREE.Vector3
in the simulation part of the app, and since many vector operations are implemented in place,.clone()
calls are very common. When I downgrade version to0.119.1
, the clone operation isn't there anymore and the model step calculates 4 times faster.I did a little investigation and replacing
Object.defineProperty( this, 'isVector3', { value: true } );
inTHREE.Vector3
constructor withthis.isVector3 = true;
fixes the issue.To Reproduce, Live example
On my machine, the difference is 5ms (r119) vs 45ms (r120, r125).
I don't think it'd be useful here, but the app I'm working on:
https://tectonic-explorer.concord.org/?preset=subduction
Expected behavior
No .clone performance drop.
Screenshots
THREE.JS version 0.119.1 vs 0.120.0:
Platform:
The text was updated successfully, but these errors were encountered: