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

Avoid using unique ID in child compiler template #1836

Merged

Conversation

helloitsjoe
Copy link
Contributor

What problem does this solve?

Unique child compiler IDs cause a memory leak in watch mode

Resolves #1835

Details

This change reverts aa64b82, which introduced a change where each child compiler is given a unique ID. This causes duplicate source strings to be cached on every recompile, without removing the old sources.

More details and screenshots in the issue linked above. The main thing I'm unclear about is why unique IDs were used in the first place, but as far as I can tell the ID isn't referenced anywhere else. Let me know if I've missed something!

Testing done

  • Unit tests pass
  • Tested locally, the memory leak is resolved when using template instead of the unique ID

@helloitsjoe helloitsjoe changed the title Avoid using unique ID in child compiler template path Avoid using unique ID in child compiler template Dec 12, 2023
@alexander-akait alexander-akait merged commit c79f2cf into jantimon:main Dec 19, 2023
19 checks passed
@alexander-akait
Copy link
Collaborator

Thank you

chenjiahan added a commit to rspack-contrib/html-rspack-plugin that referenced this pull request Dec 22, 2023
* fix: reemit favicon in serve/watch mode (jantimon#1804)

* refactor: use logger to avoid pulling logs in output (jantimon#1805)

* refactor: avoid extra file (jantimon#1806)

* refactor: code (jantimon#1807)

* docs: improve (jantimon#1808)

* fix: avoid have undefined `type` for script tags (jantimon#1809)

* refactor: fix regression after refactor and testing (jantimon#1810)

* fix: reemit assets from loaders (jantimon#1811)

* test: case for watch/serve and reemitting assets (jantimon#1812)

* docs: add js-entry-webpack-plugin (jantimon#1732)

* chore(release): 5.5.4

* docs: add html-webpack-inject-attributes-plugin to readme (jantimon#1787)

* feat: Added support `type=systemjs-module` via the `scriptLoading` option (jantimon#1822)

* fix: memoy leak (jantimon#1836)

* feat: add `@rspack/core` as an optional peer dependency (jantimon#1829)

* chore(release): 5.6.0

---------

Co-authored-by: Alexander Akait <4567934+alexander-akait@users.noreply.github.com>
Co-authored-by: liam <lawler61@163.com>
Co-authored-by: alexander.akait <sheo13666q@gmail.com>
Co-authored-by: Yuwen Duan <dyw934854565@sina.com>
Co-authored-by: 坏人日记 <badry.lin@gmail.com>
Co-authored-by: Joe Boyle <jboyle617@gmail.com>
@creage
Copy link

creage commented Jan 19, 2024

@alexander-akait @helloitsjoe this change leads to an issue - webpack-dev-server doesn't pick changes:

[webpack-dev-server] App updated. Recompiling...
[webpack-dev-server] Nothing changed

Rolling back to html-webpack-plugin version 5.5.3 fixes the issue.

@alexander-akait
Copy link
Collaborator

@creage Can you create reproducible test repo?

@creage
Copy link

creage commented Jan 19, 2024

@alexander-akait here you go https://github.com/creage/html-webpack-plugin-nothing-changed

@alexander-akait
Copy link
Collaborator

@creage Thank you, I will look soon

@helloitsjoe
Copy link
Contributor Author

@creage I cloned your reproduction repo and it looks like 5.5.4 also has the problem you describe. This change was released in 5.6.0, so it seems likely this is caused by something between 5.5.3 and 5.5.4, unless you're seeing something different.

@alexander-akait
Copy link
Collaborator

@creage Intresting, I would say that we have fixed the bug, before you always had extra compilation and spent extra resources and that is why you have reloading, but when you watch dll, dll-user doesn't really know should be the application reloaded or not, you don't have HMR and you don't watch, and so you have:

[webpack-dev-server] App updated. Recompiling...
[webpack-dev-server] Nothing changed.

Because webpack doesn't know how to apply changes. How to fix it:

watchFiles: [path.join(__dirname, "../dist/manifest.json")],

And now dev server will reload the page when manifest.json was changed.

Also you can debug it using:

stats: {
  hash: true
},

And you will see that application wasn't changed, because nothing was changed in dll-user.

Feel free to feedback

@creage
Copy link

creage commented Jan 20, 2024

@alexander-akait Well, since my app uses a dll, it becomes part of it, and changes to dll are changing my app, which should be reflected by reload. And, it used to work up to 5.5.3. I can watch for manifest, but that’s kinda hacky.

@alexander-akait
Copy link
Collaborator

@creage The manifest is just a file and just a file dependency, changes in dll didn't change your application code (i.e. dll-user), as I said above you can see hash was not changed when you done a change in dll, for example just create index.html manually and add the script tags and you will see the same behaviour, html-webpack-plugin had a bug and changed a hash, but should not do it

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.

Memory leak in watch mode due to unique child compiler IDs
3 participants