Skip to content

Conversation

piaccho
Copy link
Collaborator

@piaccho piaccho commented Jul 28, 2025

This PR modifies the useBuffer hook to simplify its implementation and improve performance.

  • Removed the useRef, replacing it with a direct management of the buffer lifecycle.
  • Replaced useLayoutEffect with useEffect for writing values to the buffer.
  • Removed the timeout logic for buffer destruction, replacing it with a simpler cleanup mechanism that directly destroys the buffer if it is not already destroyed.

Test

To test the new implementation, I added proper logs in useBuffer hook and labeled buffers from ConfettiViz component to effectively trace the buffer lifecycle and renders. As it is seen in testing, the buffer is successfully reallocated on schema change and new data is successfully written on useEffect without performance impact.

Old implementation

old-implementation.mov

New implementation

new-implementation.mov

@piaccho piaccho requested a review from iwoplaza July 28, 2025 12:05
Copy link
Collaborator

@iwoplaza iwoplaza left a comment

Choose a reason for hiding this comment

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

Awesome! One question, could you test this out in React.StrictMode? I am wondering if the interplay between useMemo and the cleanup in useEffect could cause the first buffer to be destroyed even before it's first use.

@piaccho
Copy link
Collaborator Author

piaccho commented Jul 30, 2025

@iwoplaza You were right, my implementation caused that the first buffer was destroyed on first use. This ref based implementation wasn't for no reason 😅. I tried to implement this hook without using it, but with no success. I will revert to previous implementation and replace useLayoutEffect for useEffect for handling writing new values to buffer. When it comes to allocating a buffer only on mount and reallocating it if the data schema changes - I think it worked this way.

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