Skip to content

Conversation

@fereidani
Copy link

Hey Elham, Cool project!

This optimizes your render sorting by reducing sorting size(code isn't using &Arc reference and it only increase memory swap time while sorting(2x copy time)) and also pre-allocated the buffer so it doesn't heap allocate in the program render loop.

You can remove the unsafe part yourself later by adding lifetimes.

Important Note: If you merge this, you owe me a Kabuli Pulu!

@ElhamAryanpur ElhamAryanpur self-requested a review December 17, 2025 10:03
@ElhamAryanpur ElhamAryanpur added the enhancement New feature or request label Dec 17, 2025
@ElhamAryanpur
Copy link
Collaborator

regarding the PR itself, there are some issues:

  1. The graphics are GPU bound more than CPU bound, making these types of optimizations unnecessary
  2. The Objects vary greatly in size and count depending on projects. Some may favor batch rendering and merge many objects together into one for a large render pass, while many would have incredible number of objects instead. Making assumptions on their size and caching methods gets very tricky in this case, leading to crashes if sorting buffer is off by an amount.

Alternatively, a better optimization in my opinion would be caching the sort result and checking if any change have been made. This can be done as easily as a separate field with a boolean, which again is unnecessary for its scale. Another one that can help the engine in general would be swapping the hashing function of the HashMap that objects are stored in with a faster and cryptographically unsafe hashing algorithm.

All in all, it is an amazing PR, just perhaps the issues it can bring would be larger than the benefits in majority of our userbase's usecases


The Kabuli Palu is earned regardless of merge XD

@fereidani
Copy link
Author

Hey Elham,
Like the original algorithm, It is sorting references to objects(Vec<&Object>) not objects so size of object wouldn't matter actually, and this algorithm is actually safe to use AFAIK and see, it is only sorting 8byte(64-bit) pointers which helps to reduce memory copies and also fits in cpu cache much better than what it was before.

About the caching, you can add add a generation token, update that generation token whenever a object is removed or added to the ObjectStorage.
Now cache the sorted objects and reuse them if ObjectStorage token matches sorted objects token, regenerate the sorted list using same algorithm if token is changed.

Best way to implement it is to move sorting logic to ObjectStorage implementation.

Apart these:
The Kabuli Pulu argument was the most important thing that is now resolved

@ElhamAryanpur
Copy link
Collaborator

This is really interesting, I'll do a stress test this coming weekend, will report here once done.

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

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants