pathUtils | fix memory leak

webpack / enhanced-resolve

Offers an async require.resolve function. It's highly configurable.

MIT License

916 stars 186 forks source link

pathUtils | fix memory leak #418

Open Galcarmi opened 2 months ago

Galcarmi commented 2 months ago

Closes https://github.com/webpack/enhanced-resolve/issues/414

linux-foundation-easycla[bot] commented 2 months ago

:x: - login: @Galcarmi / name: Gal carmi . The commit (86fd6ca0dcbce9536bdccb26251efd70177a1c73, 6b55c49e6cdd76fbc2cb565055481581e360c257, 0d6d26b95a5c25cc9d6a700d700b242775d6d6b7, b7dcd3e97548ffd042797efadc74168cb6b81fc4) is not authorized under a signed CLA. Please click here to be authorized. For further assistance with EasyCLA, please submit a support request ticket.

alexander-akait commented 1 month ago

@vankop What do you think about it? Yeah, we have a memory leak here, if you run develpment your application very long our cache will be bigger and bigger, so I think using lru cache is good idea, but I don't know what is will be better number for max

vankop commented 1 month ago

I personally think we dont need cache here.. maybe at first implementation there was some specific usage for resolver.join, but right now it used in a lot of cases, sometimes even on each plugin call. ( see JoinPlugins ) So probably cache hit is very low. basically happen only on the same resolving, this happens quite rarely on incremental builds

vankop commented 1 month ago

on cold builds it does not make sense either since very low cache hit

Galcarmi commented 1 month ago

@alexander-akait sorry I'm a bit busy, I'll address it at the weekend,

@vankop so do you recommend to just remove it instead?

alexander-akait commented 1 month ago

@vankop Can you test it on some repo and check it?

vankop commented 1 month ago

we have a metrics repo, we just need to setup cache hits there...