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

Performance overhead in proof_trace_callback_writer #1197

Open
stevenmeker opened this issue Jan 31, 2025 · 1 comment · Fixed by #1198
Open

Performance overhead in proof_trace_callback_writer #1197

stevenmeker opened this issue Jan 31, 2025 · 1 comment · Fixed by #1198

Comments

@stevenmeker
Copy link
Collaborator

The MPG team have reported a substantial overhead with this class, even when the callback functions are empty. The code in this class does very little but makes use of std::vector<> instantiations for function symbol arguments and substitutions. Putting data in a freshly allocated std::vector<> object implies a heap allocation which much be handled by malloc() and deallocated by free(). One or more malloc()/free() call pairs per rewrite would likely account for the overhead.

One possible resolution is to have a class in the MPG inherit directly from the abstract base class, proof_trace_writer, and cut out proof_trace_callback_writer altogether.

An alternative resolution is to have the needed std::vector<> objects allocated just once inside proof_trace_callback_writer and then resized and reused as needed for each callback. Resizing a vector that already has sufficient memory attached is cheap. Futhermore std::vector<> objects can be moved efficiently with std::move(). However this means that data structures can only be loaned to the MPG in a callback and will not be valid after the callback returns, because they will have be moved or reused.

@stevenmeker
Copy link
Collaborator Author

Actually there are data members in proof_trace_callback_writer that are being reused.

  std::optional<call_event_construction> current_call_event_;

  std::optional<rewrite_event_construction> current_rewrite_event_{
      std::nullopt};

The problem is that the are being reused by calling the optional::reset() method that destroys the contained object and optional::emplace() method that creates a new one. Destroying and creating std::vector<> instances is expensive.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
1 participant