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

Update add/modify/remove sim methods to use safe version #96

Open
nreinicke opened this issue Aug 22, 2022 · 3 comments
Open

Update add/modify/remove sim methods to use safe version #96

nreinicke opened this issue Aug 22, 2022 · 3 comments
Milestone

Comments

@nreinicke
Copy link
Collaborator

nreinicke commented Aug 22, 2022

PR #95 adds in new safe methods for adding/modifying/removing hive entities from the simulation state. Now, we need to go through and update the usage of the old methods to use these new safe methods internally.

For example, in

error, updated_sim = simulation_state_ops.modify_request(sim, updated_request)
we could update this to use the safe method modify_request_safe().

@robfitzgerald
Copy link
Collaborator

robfitzgerald commented Aug 23, 2022

i'd like to couple with this a performance benchmark, to make sure the overhead of switching to ResultE is insignificant.

edit: and yes i would also like this!

@nreinicke
Copy link
Collaborator Author

i'd like to couple with this a performance benchmark, to make sure the overhead of switching to ResultE is insignificant.

yes, that's a great point; this would be great to have but not if it comes at a high performance cost.

@nreinicke
Copy link
Collaborator Author

I just did a quick test of this, comparing a method that returns the old ErrorOr type to a function that returns the ResultE type (and unwraps the result). The performance cost for this is ~1 microsecond. This seems like an okay tradeoff in performance relative to what we gain in ergonomics.

@nreinicke nreinicke added this to the PyCon 2023 milestone Apr 11, 2023
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

No branches or pull requests

2 participants