Skip to content

Conversation

capdevon
Copy link
Contributor

@capdevon capdevon commented Jun 17, 2025

Reduce object allocations when ALAudioRenderer update listener each time the Camera is moved:

This pull request refactors the Listener class in the jMonkeyEngine audio system. The main changes include initializing the location, velocity, and rotation fields at declaration instead of in the constructor, removing redundant constructor assignments, and introducing reusable Vector3f instances (left, up, direction) for direction calculations.

Methods that return direction vectors now store results in these fields rather than creating new objects, improving memory efficiency and reducing unnecessary allocations. Overall, the changes streamline object management and optimize performance in the Listener class.

https://github.com/jMonkeyEngine/jmonkeyengine/blob/master/jme3-core/src/main/java/com/jme3/audio/openal/ALAudioRenderer.java#L768

    private void applyListenerRotation(Listener listener) {
        Vector3f dir = listener.getDirection(); // <--
        Vector3f up = listener.getUp(); // <--
        // Use the shared FloatBuffer fb
        fb.rewind();
        fb.put(dir.x).put(dir.y).put(dir.z);
        fb.put(up.x).put(up.y).put(up.z);
        fb.flip();
        al.alListener(AL_ORIENTATION, fb);
    }

@yaRnMcDonuts yaRnMcDonuts added this to the v3.9.0 milestone Jun 19, 2025
*/
public Vector3f getLeft() {
return rotation.getRotationColumn(0);
return rotation.getRotationColumn(0, left);
Copy link
Member

@richardTingle richardTingle Jun 26, 2025

Choose a reason for hiding this comment

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

This changes the behaviour. Previously this returned a fresh Vector3f that you had exclusive ownership of. Now you are returned a vector3f that may randomly change at some future point.

Unless this is giving a major boost in performance (from profiling) in typical flows I don't think we should do this

Copy link
Contributor Author

@capdevon capdevon Jun 27, 2025

Choose a reason for hiding this comment

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

This solution maintains consistency with the existing getLocation(), getRotation(), and getVelocity() methods, which already expose internal variables. This approach is not only consistent but also crucial for performance optimization and efficient reuse of internal variables, preventing new vector allocations every time.

The primary reason for this approach is that Listener is an internally managed class by ALAudioRenderer and AudioListenerState. It's designed as a unique instance created by LegacyApplication and is not intended for external direct use. Therefore, the concerns you've raised regarding its internal exposure are mitigated by its strictly internal scope and established usage patterns.

Edit:
Furthermore, specifically within ALAudioRenderer.applyListenerRotation(), having redundant copies of up and direction variables is unnecessary. The underlying audio buffer only needs to read the x, y, and z coordinates directly from the existing variables. Creating and passing extra copies would just add needless overhead.

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.

3 participants