Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Classify and cleanups #12

Open
wants to merge 7 commits into
base: development
Choose a base branch
from
Open

Conversation

tiennou
Copy link

@tiennou tiennou commented May 31, 2024

This comes from me trying to integrate it in OM, since it has its own segment manager, handles instantiation, etc.

The big changes are that any sort of segment access goes through protected methods (so supposedly you just subclass it and provide your own stuff), and loads the segments in a round-robin, so you can always look at the last state you saw from an ally. The rest is mostly style cleanups.

Better look at the individual commits. I can definitely slice and dice stuff if it feels not appropriate (I realize writing this that caching all that stuff makes it hard on the heap usage, for example).

This turns the segment reading into a cached round-robin, as it will
now load the segment in the background and save its value in memory.

The ally list can now be changed dynamically through a couple of
accessors as well.
@CarsonBurke
Copy link
Member

CarsonBurke commented Jun 17, 2024

I want to like this but I have concerns. Caching all the ally data is going to add a lot of heap. Have you done profiling on potential heap usage increases, coinciding gc and cpu costs? @tiennou

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