Skip to content

Conversation

AlexAPPi
Copy link
Contributor

@AlexAPPi AlexAPPi commented Oct 3, 2025

Description

Replaced dynamic buffer allocation with pre-typed memory structures
to minimize runtime overhead and reduce GC pressure. This improves
wireframe index generation performance by eliminating type checks
and avoiding frequent temporary allocations.

I have read the contributing guidelines

Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR optimizes wireframe index generation by replacing dynamic array allocation with pre-typed memory structures to reduce runtime overhead and GC pressure.

Key changes:

  • Replaced dynamic lines array with pre-allocated typed arrays
  • Eliminated intermediate array operations by writing directly to typed array buffers
  • Optimized buffer creation by passing the typed array buffer directly to IndexBuffer constructor

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@mvaligursky
Copy link
Contributor

AI review, which it seems to make sense after a quick look:

Assessment
Pros:
✅ Eliminates dynamic array growth overhead
✅ Removes one copy operation (direct buffer passing)
✅ Better for performance-critical scenarios
✅ Handles edge case where vertices aren't divisible by 3
Concerns:
⚠️ Indexed path memory inefficiency: srcIndices.slice(0) allocates a full copy of the source index buffer even though typically only a fraction is needed (unique edges << total indices)
⚠️ Code is less readable
⚠️ For small meshes, the optimization benefit may be negligible compared to complexity cost
Recommendation
The non-indexed path optimization is excellent and should definitely be applied.
For the indexed path, I'd suggest an alternative: pre-allocate based on worst-case (count * 2) instead of copying the entire source buffer:
This would be more memory-efficient while keeping the performance benefits.

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.

2 participants