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

add test for #5395 #5400

Closed

Conversation

Kilowhisky
Copy link

Adds failing test for #5395

@runspired
Copy link
Contributor

As ember has deprecated array observers in 4.0, and as there are many patterns for reacting to record-array changes efficiently without array-observers, and as the relationship layer has been refactored to generally be far more efficient causing over notifications to go down (the source of some of the trouble leading to the issue for which this PR was opened), and as the craziness of unloadRecord is mostly being eliminated due to internal conversion to identifiers (such that unloaded records can always be fully released), I'm going to close this.

Long winded I know but that's because fundamentally I care about why this PR was opened in the first place: performant array operations matter a lot. There are a lot of good patterns out there though that don't require the array-observer, and we're dedicated to improving base performance where possible. If @Kilowhisky or anyone else watching this issue still faces performance problems using octane paradigms and ember-data 3.28+ please reach out. At a minimum we should create a performance benchmark to add to the performance tests we run in CI so that we can monitor and focus on improving the base performance of the scenario, and if it's evident more is needed to achieve the necessary performance characteristics for large data sets then we'll look into what we can do to make subscribing to granular updates for a type first-class.

@runspired runspired closed this May 26, 2021
@runspired runspired added 🏷️ test This PR primarily adds tests for a feature and removed test-contribution labels Sep 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏷️ test This PR primarily adds tests for a feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants