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.

Sign up for GitHub

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

Jump to bottom

Optimize calculation/aggregate/relationship merging/reuse in calculation dependencies #1282

Open
zachdaniel opened this issue Jul 4, 2024 · 0 comments
Open

Optimize calculation/aggregate/relationship merging/reuse in calculation dependencies #1282

zachdaniel opened this issue Jul 4, 2024 · 0 comments
Labels
enhancement New feature or request

Comments

@zachdaniel
Copy link
Contributor

We had to make a recent fix that reduces drastically the amount of cases where calculations, relationships and aggregates required by calculations in their load/3 callback can be reused in the case that they are being loaded. This is because there is an important semantic difference: calculation dependencies should not have authorization rules applied to them.

However, there should be some analysis that can be applied to determine the ability to share, for example:

  • if the calculation is being run with calculate/3, we can reuse it.
  • If the calculation doesn't reference any aggregates or relationships, we can reuse it (traversing any nested calculations)
  • If the calculation references relationships and those relationships have no authorizers

These are some naive optimizations that could be applied.
@zachdaniel zachdaniel added the enhancement New feature or request label Jul 4, 2024
rbino added a commit to rbino/edgehog that referenced this issue Jul 10, 2024
@rbino
Update deps
cf16fcc 
There was a new ash and ash_postgres minor release, which includes useful
features for development (namely: generators), so include that.
Note that we have to do a slight change to tests since due to
ash-project/ash#1282 the location calculation is not
reused when querying the position, so the test should expect 2 calls to the
mock. This could possibly change in the future when it is optimized again, so
leave a TODO detailing this.
@rbino rbino mentioned this issue Jul 10, 2024
Merged
rbino added a commit to rbino/edgehog that referenced this issue Jul 10, 2024
@rbino
Update deps
561fd70 
There was a new ash and ash_postgres minor release, which includes useful
features for development (namely: generators), so include that.

Note that we have to do a slight change to tests since due to
ash-project/ash#1282 the location calculation is not
reused when querying the position, so the test should expect 2 calls to the
mock. This could possibly change in the future when it is optimized again, so
leave a TODO detailing this.

Signed-off-by: Riccardo Binetti <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
enhancement New feature or request
Projects
Ash Framework Roadmap
Status: Soon
Milestone
No milestone
Development

No branches or pull requests
1 participant
@zachdaniel