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

Wrap the state to navigate the problem space of remove_finished_globals into a struct and again into a std::shared_ptr. #1852

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

copybara-service[bot]
Copy link

Wrap the state to navigate the problem space of remove_finished_globals into a struct and again into a std::shared_ptr.

Profile shows that the majority of the CPU time is spent in construction and destruction of the tree (GoalSet --> std::set<..>), which is mostly caused when doing queue.pop_front();. but if we put this data in a shared_ptr instead, we won't delete it immediately, and the heap allocated data will be reused in the case of continue statements, that just puts back the state into the queue without any modification. This change is something that has no downside but has only upsides.

Possibly there would be a cleverer fix that would make the internal data more reusable in case of the state-branching within the for loop, but this is an improvement which would be helpful with a quicker fix.

…als` into a `struct` and again into a `std::shared_ptr`.

Profile shows that the majority of the CPU time is spent in construction  and destruction of the tree (`GoalSet` --> `std::set<..>`), which is mostly caused when doing `queue.pop_front();`. but if we put this data in a shared_ptr instead, we won't delete it immediately, and the heap allocated data will be reused in the case of continue statements, that just puts back the state into the queue without any modification. This change is something that has no downside but has only upsides.

Possibly there would be a cleverer fix that would make the internal data more reusable in case of the state-branching within the for loop, but this is an improvement which would be helpful with a quicker fix.

PiperOrigin-RevId: 704637937
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.

1 participant