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

Reassign state Var when fields on a Base instance change #1748

Merged
merged 8 commits into from
Sep 18, 2023

Conversation

masenf
Copy link
Collaborator

@masenf masenf commented Sep 4, 2023

Refactor mutable type handling in State classes:

  • Create new MutableProxy object that wraps list, dict, set, and Base-derived objects coming from the State.
  • When certain __magic_attrs__ are accessed on wrapped objects, and they are callable, mark the associated var field in the state as dirty.
  • When accessing attributes or items through the proxy, and the resulting object is also mutable, then also wrap it in MutableProxy associated with the original state and field_name.
  • When assigning a MutableProxy back to the state, actually assign the wrapped object.

This approach does not incur as much runtime overhead as the current implementation, since it does not create copies of objects just for state tracking and does not create copies of objects when assigning back to the state. Instead it just wraps the original mutable object.

Fixes #1675

@masenf masenf requested a review from picklelo September 4, 2023 18:28
@masenf
Copy link
Collaborator Author

masenf commented Sep 5, 2023

Yikes, this appears to approximately double the time taken by tests/test_state.py 😱

@masenf masenf marked this pull request as draft September 5, 2023 04:09
@masenf
Copy link
Collaborator Author

masenf commented Sep 15, 2023

So I was thinking about this one a bit more... and instead of doing a full re-assignment (and reinitialization, etc) for these mutable types, we probably should just mark them dirty in the state.

Essentially, we will add some code to State.__getattr__ which returns an object proxy for mutable types, when any attribute is set or mutation method called, the proxy will mark the field as dirty in the state, and then pass the call through to the underlying object.

Then State.__setattr__ will be extended to identify when attempting to assign an object proxy and just assign the object itself instead.

With this approach, we won't need specialty collection objects and we won't need slow up-front depth first traversals. We can just mark the fields appropriately at runtime when they are accessed/modified.

@masenf masenf force-pushed the masenf/custom-var-reactivity branch from ea49999 to 5ad6c01 Compare September 16, 2023 07:35
@masenf masenf force-pushed the masenf/custom-var-reactivity branch from 5f337a0 to 582031b Compare September 16, 2023 08:07
@masenf masenf marked this pull request as ready for review September 16, 2023 08:25
@masenf masenf requested a review from Lendemor September 16, 2023 08:36
Copy link
Contributor

@picklelo picklelo left a comment

Choose a reason for hiding this comment

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

nice, this solution ended up very clean :)

@picklelo picklelo merged commit 1430075 into main Sep 18, 2023
35 checks passed
@picklelo picklelo deleted the masenf/custom-var-reactivity branch October 9, 2023 21:07
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.

[REF-619] Custom Vars do not update after changing one of the value.
2 participants