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 improvement on (nearly) all exports #3727

Closed
cielf opened this issue Jul 9, 2023 · 7 comments · Fixed by #3739
Closed

Performance improvement on (nearly) all exports #3727

cielf opened this issue Jul 9, 2023 · 7 comments · Fixed by #3739
Assignees
Labels

Comments

@cielf
Copy link
Collaborator

cielf commented Jul 9, 2023

Summary

We recently improved the performance on distribution exports by an order of magnitude. Apply that change to the other exports

Details

This change makes the item_header an O(n) instead of an O(n**2), which makes a huge difference in production.

For details of the change, see the pull request Memoize item headers to speed up export #3581
The following exports may need to be improved in this way:

  • purchases
  • donations
  • storage locations
  • requests
  • product drives

Criteria for completion

-[ ] the above functionality is in place
-[ ] the complete test suite passes

@cielf cielf added Help Wanted Groomed + open to all! Difficulty—Beginner labels Jul 9, 2023
@jadekstewart3
Copy link
Contributor

May I try my hand at this?

@dorner
Copy link
Collaborator

dorner commented Jul 10, 2023

Go for it!

@jadekstewart3
Copy link
Contributor

@dorner Would it be wise to benchmark my changes to ensure its faster? I've never tested for speed before, and I'm curious if benchmarking would be the best way to go about it?

@dorner
Copy link
Collaborator

dorner commented Jul 13, 2023

Hmm... not sure it's necessary for this. Benchmarking is really when you're trying to look at micro-speedups, i.e. when you have to run things many times and measure total time which might be close. In this case we're looking at speeding things up probably by an order of magnitude - you should see very quickly if your solution was successful.

@cielf
Copy link
Collaborator Author

cielf commented Jul 13, 2023

It might not be quite so blindingly obvious on the seed data -- there just isn't that much.

@dorner
Copy link
Collaborator

dorner commented Jul 13, 2023

Yeah this definitely should be run on a production copy if possible.

@cielf
Copy link
Collaborator Author

cielf commented Jul 13, 2023

I can fit in time to do that as a check before we merge

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

Successfully merging a pull request may close this issue.

3 participants