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

Do not needlessly access isDirty #1298

Merged
merged 3 commits into from Sep 1, 2021
Merged

Do not needlessly access isDirty #1298

merged 3 commits into from Sep 1, 2021

Conversation

igorT
Copy link
Collaborator

@igorT igorT commented Aug 31, 2021

We were entangling isDirty by doing a get(this, 'isDirty'); even when no one had read our dirty state. In some edge cases, this could result in rerendering errors, and was also not great for performance.

@github-actions
Copy link

github-actions bot commented Aug 31, 2021

Performance Report for 00f4796

Scenario - materializing: ⚠️ Performance regressed

⚠️ duration
phase estimated regression +41ms [14ms to 75ms] OR +0.85% [0.29% to 1.53%]
☑️ Phase [navigationStart] => [start-loading]
phase no difference [-3ms to 5ms]
☑️ Phase [start-loading] => [pushed-payload]
phase no difference [-10ms to 27ms]
⚠️ Phase [pushed-payload] => [end-loading]
phase estimated regression +35ms [21ms to 50ms] OR +1.83% [1.11% to 2.64%]
☑️ Phase [end-loading] => [Test End]
phase no difference [0ms to 1ms]

Scenario - rendering: ☑️ Performance is stable

☑️ duration
phase no difference [-8ms to 4ms]
☑️ Phase [navigationStart] => [start-loading]
phase no difference [-2ms to 5ms]
☑️ Phase [start-loading] => [pushed-payload]
phase no difference [-1ms to 0ms]
☑️ Phase [pushed-payload] => [end-loading]
phase no difference [0ms to 0ms]
☑️ Phase [end-loading] => [Test End]
phase no difference [-5ms to 2ms]

@igorT igorT added the bug ch:bugfix label Aug 31, 2021
Co-authored-by: Steven Pham <2080348+spham92@users.noreply.github.com>
addon/model.js Outdated Show resolved Hide resolved
@igorT igorT requested a review from hjdivad August 31, 2021 23:32
@igorT igorT dismissed hjdivad’s stale review September 1, 2021 20:34

addressed feedback

@igorT igorT merged commit 8f769fd into master Sep 1, 2021
@igorT igorT deleted the igor/fix-isDirty branch September 1, 2021 21:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug ch:bugfix
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants