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

Optimize visiting the relating records #7096

Merged
merged 1 commit into from
Jul 2, 2020
Merged

Optimize visiting the relating records #7096

merged 1 commit into from
Jul 2, 2020

Conversation

pieter-v
Copy link
Contributor

Visiting all related records during _cleanupOrphanedRecordDatas allocates and copies a large amount of unnecessary data. Using an Iterable this allocation is not done anymore.
In one of our applications we have a large dataset with complex relations. Destroying a large number of records timed out after 10 minutes, with this fix it still takes two minutes (but it is already a big improvement)

@runspired
Copy link
Contributor

@pieter-v I'm a bit less certain about this change without also adding a perf test. Will likely also want to look at it running manually.

@igorT
Copy link
Member

igorT commented Jun 30, 2020

@pieter-v Thanks so much for this deep dive! @runspired it seems like we landed more perf tests, is there anything blocking this?

@runspired
Copy link
Contributor

@igorT all we need to do is rebase so that the new performance test is run so that we can see the output from this change, I am otherwise 👍 on merging this.

@pieter-v
Copy link
Contributor Author

pieter-v commented Jul 2, 2020

@runspired @igorT rebase done

@runspired
Copy link
Contributor

Performance check indicates a ~6500ms improvement for the unloading scenario, with other scenarios unchanged.

@runspired runspired merged commit da70e6c into emberjs:master Jul 2, 2020
@pieter-v pieter-v deleted the destroy-optimize branch July 28, 2020 08:11
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.

None yet

4 participants