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

queryParams + tracked performance issue #18715

Closed
Windvis opened this issue Jan 30, 2020 · 21 comments
Closed

queryParams + tracked performance issue #18715

Windvis opened this issue Jan 30, 2020 · 21 comments

Comments

@Windvis
Copy link
Contributor

Windvis commented Jan 30, 2020

We noticed some performance issues when decorating query param properties with @trackedand using it in a template. Removing tracked (and using set instead) fixes the problem. Removing the property name from the queryParams array also fixes the issue.

I'll try to create a reproduction project, but it's not noticeable in clean / small projects. I'll investigate more when I find some time.

When searching the discord I discovered there are more people who have this problem, but it seems no issue was logged yet.

Relevant Discord discussions:
https://discordapp.com/channels/480462759797063690/608346628163633192/638740754650628098

@williamweckl
Copy link

I've noticed this issue as well. Exactly the same behavior you described..

@rwjblue
Copy link
Member

rwjblue commented Feb 5, 2020

Hmm, since it isn't completely obvious in a smaller app scenario I'm not sure how to dig into this. Does someone have time to make a reproduction that we could use (with a quick bulleted list of steps to follow to see the issue)?

@mydea
Copy link
Contributor

mydea commented Feb 19, 2020

In our app, setting @tracked on a qp made the page basically unusable in Firefox. It worked fine in Chrome and Safari, though. But in Firefox, as soon as the property was set, doing anything lead to a FPS drop to about 1-3 FPS for a second or so.

@broerse
Copy link

broerse commented Feb 19, 2020

I use @Tracked query params here but don't see slowness. I had to create queryParamsObj to pass the query params to components see:

https://github.com/broerse/ember-cli-blog/blob/master/app/controllers/posts.js#L8

@andrewfan
Copy link

faced this issue as well, everything hangs here @glimmer/reference.js MonomorphicTagImpl.validate basically revision tag goes up to billions and I see that tag.subtags always have more subtags and so on

kevinhinterlong pushed a commit to yavin-dev/framework that referenced this issue Mar 6, 2020
- Query params are technically owned (managed) by the ember framework
 so we should be using set to notify them to update
- https://discuss.emberjs.com/t/query-params-tracked/17467/21
- emberjs/ember.js#18715
kevinhinterlong added a commit to yavin-dev/framework that referenced this issue Mar 9, 2020
- Query params are technically owned (managed) by the ember framework
 so we should be using set to notify them to update
- https://discuss.emberjs.com/t/query-params-tracked/17467/21
- emberjs/ember.js#18715

Co-authored-by: Kevin Hinterlong <hinterlong@verizonmedia.com>
@misterbyrne
Copy link

We noticed this happening too. The performance became massively degraded when we added a second @tracked property bound to a queryParam (any browser became unusable). We had to work around it by using a normal property using Ember.set instead.

@stukalin
Copy link
Contributor

Interestingly I also don't see this issue even though I had to add a bunch of @tracked QPs. Tested on Chrome, FF, Edge, IE11. We're on 3.16 now...

@jrjohnson
Copy link
Sponsor

jrjohnson commented Mar 25, 2020

I was also unsuccessful in creating a limited reproduction for this, but I've created a working reproduction with a minimal change set in this PR ilios/common#1269 (ember 3.17)

Steps to tests:

  1. checkout
  2. create .env file with ILIOS_FRONTEND_API_HOST=https://ilios3-demo.ucsf.edu
  3. npm install, npm start, visit page in firefox
  4. Reach out to me on discord (@jrjohnson) and I'll give you a token, it's demo data so we're happy to make it available, just not to the whole world.

Once authenticated you'll see our dashboard.

  1. Click on Calendar
  2. Click on Show Filters
    2a) Sometimes this will fail here, the filters will keep trying to load and spinning, eventually the browser will crash.
  3. Start clicking on checkboxes. You'll see that the URL updates immediately, but it takes a few moments for the checkbox to update. As your machine gets slower it will take longer. Performance seems to get worse over time, but sometimes starts out bad.

@kolomeetz
Copy link

I was bitten by this yesterday, spent quite a time trying to understand what was going on. Ember 3.14.3, Firefox and Chrome.

@njoyard
Copy link

njoyard commented Apr 5, 2020

Running into this as well, managed to find a workaround using a tracked backing property and getter/setter:

  queryParams = ['prop']

  @tracked _prop = 'default'

  get prop() {
    return this._prop
  }

  set prop(value) {
    this._prop = value
  }

@lvegerano
Copy link

Running into this problem as well. Props to @njoyard for the workaround. Using Ember 3.14.3.

@chrisvdp
Copy link

chrisvdp commented Jun 4, 2020

We are also running into this, @njoyard workaround fixed the issue. Using Ember 3.16.6

@vst
Copy link

vst commented Jul 1, 2020

Same here:

  • Was using Ember 3.16, upgraded to Ember 3.19. Nothing has changed as far as I could diagnose.
  • Using TypeScript.
  • There are a few tracking properties of which one yields a promise.
  • @njoyard workaround fixed the issue.

I have noticed considerable slowness. But I understood that there was something very wrong when I saw hundreds of [Violation] 'setTimeout' handler took XXXms messages on the Chrome devtools console.

@mcfiredrill
Copy link
Contributor

I have noticed considerable slowness. But I understood that there was something very wrong when I saw hundreds of [Violation] 'setTimeout' handler took XXXms messages on the Chrome devtools console.

Noticing similar issues with lots of warning messages like this.

@charlesfries
Copy link

+1 Ember 3.19, tested in Chrome (84.0.4147.105) and Firefox (78.0.2), only occurs in Firefox

@Samsinite
Copy link

@rwjblue do you know if #19138 fixes this issue?

@pzuraq
Copy link
Contributor

pzuraq commented Oct 7, 2020

@Samsinite we don't know for sure since we never got a reproduction I guess I just never got the memo that we did get one 🤦 thank you @jrjohnson, but I do believe this will fix that issue

@Samsinite
Copy link

@pzuraq sorry, I should have worded that better. With insight and knowledge of the changes in that PR, and what they affected, do you think the fix could have fixed some or most of the performance regressions reported in this issue?

@pzuraq
Copy link
Contributor

pzuraq commented Oct 7, 2020

Yes, I believe so. There was one such performance issue that we found in our app that the fix resolved, and I think it's likely that it will fix other performance issues as well.

@rwjblue
Copy link
Member

rwjblue commented Oct 7, 2020

Yes, I believe that #19138 fixed this. Closing for now (can reopen if we get a repro that shows it is still an issue)..

@rwjblue rwjblue closed this as completed Oct 7, 2020
@jrjohnson
Copy link
Sponsor

Yes!! My issue and pretty broad reproduction in #18715 (comment) is resolved in Ember 3.22.0. After updating I no longer see the delays and crashing I was seeing before. 🎉!! Thanks!

MikiDi added a commit to kanselarij-vlaanderen/frontend-kaleidos that referenced this issue Mar 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests