Gamelab: Improve performance by creating fewer functions #14830
Conversation
Use simple for loops instead of forEach() calls, which eliminates: - Creation of two functions inside the draw loop - Two function invocations inside the draw loop - Temporary `self` variables.
| const preMethods = this._registeredMethods.pre; | ||
| for (let i = 0; i < preMethods.length; i++) { | ||
| preMethods[i].call(this); | ||
| } |
There was a problem hiding this comment.
This change and the one directly below I'm not sure will have a big impact, but they seem worthwhile since they both exist inside our draw loop so they happen a lot.
I'm replacing a forEach() call with a regular for loop because it allows us to create one less function, and make n less function invocations, per draw call. Because this eliminates a layer of scope we can also get rid of the unneeded self temporary variable.
| */ | ||
| s._syncAnimationSizes = function (animations, currentAnimation) {}; | ||
|
|
||
| Object.defineProperty(s, 'frameDelay', { |
There was a problem hiding this comment.
sprite.frameDelay here is a simple property alias into sprite.animation.frameDelay - and we've already got a nice system for Game Lab Sprite property aliases, so I'm using that.
| // Game Lab should have access to a scaled version. | ||
| s.getScaledHeight = function () { | ||
| return s.height * s.scale; | ||
| }; |
There was a problem hiding this comment.
There's a lot to take in here, but the general pattern is that for each of these functions we've added or overridden on the p5.play sprite, we were creating a new Function object for every single sprite instead of having one function that all sprites could refer to. I've remedied this by moving the function implementations into the module scope (see below) and just assigning them to the created sprite here. This has the added benefit that there's a little less scope magic going on there, and it's easier to read the createSprite method from start to end.
| return s.height * s.scale; | ||
| }; | ||
| // Attach our custom/override methods to the sprite | ||
| s.setAnimation = setAnimation.bind(s, this); |
There was a problem hiding this comment.
In a few cases we still have to create a function for every sprite, such as setAnimation because we bind against a particular p5 instance, so we're not looking at any performance improvement from these. Still, I think it's cleaner to put the implementation elsewhere and just do the binding here.
| s.collide = collide; | ||
| s.displace = displace; | ||
| s.bounce = bounce; | ||
| s.play = play; |
There was a problem hiding this comment.
Another nice side effect of this structure is that it's really obvious how little ordering matters for all these assignments.
| } | ||
| } | ||
| get: getWidth, | ||
| set: setWidth |
There was a problem hiding this comment.
Even for these Object.defineProperty calls, the getters and setters are generic, not bound to a particular sprite. We don't need to create them every time.
Starting with some small potential optimizations I noticed while Caley and I were looking at performance last week, I did some additional profiling and discovered that we create way too many new functions in Game Lab, especially during sprite creation. This is a series of improvements (commented inline) that should provide small performance gains, and that I think usually give cleaner code anyway.