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

perf: remove assets from stats #3905

Closed
wants to merge 1 commit into from
Closed

Conversation

markjm
Copy link

@markjm markjm commented Sep 30, 2021

  • This is a bugfix

For Bugs and Features; did you add new tests?

no, I am willing to, but am unsure exactly where this functionality is currently being tests

Motivation / Use-Case

stats.toJson call is more expensive with assets enabled, but it is not needed. Instead, we still keep track of hash so as to keep to only refresh the page when there is a real change (saving file with no changes will not refresh the page)

Before
image

After
image

Breaking Changes

none

Additional Info

@markjm
Copy link
Author

markjm commented Sep 30, 2021

cc @alexander-akait here are the changes with snippets of the traces. Saves a little under 100ms in my project

}

static get DEFAULT_STATS() {
return {
all: false,
hash: true,
assets: true,
assets: false,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we can omit this because all: false

@alexander-akait alexander-akait mentioned this pull request Oct 2, 2021
6 tasks
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

2 participants