WarningImHack3r / npm-update-dependencies

Update NPM dependencies from your IDE
https://plugins.jetbrains.com/plugin/21105-npm-update-dependencies
9 stars 2 forks source link

HTTP Caching #135

Closed SCjona closed 1 month ago

SCjona commented 1 month ago

Issue type

performance improvement

Description

The plugin performs repeated HTTP requests which can be cached (URL and time based).

See this log (code modified to see HTTP GET requests) ``` 2024-09-26 11:33:07,754 [ 145098] FINE - #com.github.warningimhack3r.npmupdatedependencies.backend.engine.NPMJSClient - HTTP GET https://registry.npmjs.org/@mui/icons-material/latest 2024-09-26 11:33:08,436 [ 145780] FINE - #com.github.warningimhack3r.npmupdatedependencies.backend.engine.NPMJSClient - HTTP GET https://registry.npmjs.org/@mui/icons-material/latest 2024-09-26 11:33:13,038 [ 150382] FINE - #com.github.warningimhack3r.npmupdatedependencies.backend.engine.NPMJSClient - HTTP GET https://registry.npmjs.org/@mui/material/latest 2024-09-26 11:33:13,732 [ 151076] FINE - #com.github.warningimhack3r.npmupdatedependencies.backend.engine.NPMJSClient - HTTP GET https://registry.npmjs.org/@mui/material/latest 2024-09-26 11:33:14,304 [ 151648] FINE - #com.github.warningimhack3r.npmupdatedependencies.backend.engine.NPMJSClient - HTTP GET https://registry.npmjs.org/@types/node/latest 2024-09-26 11:33:15,235 [ 152579] FINE - #com.github.warningimhack3r.npmupdatedependencies.backend.engine.NPMJSClient - HTTP GET https://registry.npmjs.org/@types/node/latest 2024-09-26 11:33:16,330 [ 153674] FINE - #com.github.warningimhack3r.npmupdatedependencies.backend.engine.NPMJSClient - HTTP GET https://registry.npmjs.org/@types/react-signature-canvas/latest 2024-09-26 11:33:17,024 [ 154368] FINE - #com.github.warningimhack3r.npmupdatedependencies.backend.engine.NPMJSClient - HTTP GET https://registry.npmjs.org/@types/react-signature-canvas/latest 2024-09-26 11:33:19,101 [ 156445] FINE - #com.github.warningimhack3r.npmupdatedependencies.backend.engine.NPMJSClient - HTTP GET https://registry.npmjs.org/libphonenumber-js/latest 2024-09-26 11:33:19,603 [ 156947] FINE - #com.github.warningimhack3r.npmupdatedependencies.backend.engine.NPMJSClient - HTTP GET https://registry.npmjs.org/libphonenumber-js/latest ```

Special case

SCjona commented 1 month ago

@WarningImHack3r maybe you should consider using a lib for caching e.g. https://github.com/google/guava/wiki/CachesExplained

WarningImHack3r commented 1 month ago

I wasn’t willing to use a (heavy?) library just for that and caching one HTTP request tbh If I wasn't already caching the results, yeah, it could've been more worth it, but I feel like my quick and dirty map solution is enough for this very double /latest request problem. Don't you think? What would a library bring me more for this use case?

SCjona commented 1 month ago

there's also https://github.com/ben-manes/caffeine (865kb jar lib). I think the benefit is mostly that time based expiration is handled for you and max size so you don't end up using too much memory. Also I personally prefer this builder callback approach in my own code, but that's just my preference.

And you could use that for all your caching pretty much, you just define cache keys and expiration time with these.

WarningImHack3r commented 1 month ago

Alright, this one seems worth giving a shot; I'll play around with it

I won't/can't replace my object caching with that, though. It's just going to be another layer of caching that will happen to solve the /latest duping problem as well

SCjona commented 1 month ago

I won't/can't replace my object caching with that, though

why not?

WarningImHack3r commented 1 month ago

why not?

Because it's meant to cache HTTP responses, not to store all the data I compute in memory into it

I mean, yeah, both share similarities and a very similar underlying structure (HashMap vs ConcurrentHashMap), but I feel like you're not meant to use a cache for this purpose and that you're meant to use it almost exclusively for HTTP stuff. (This might be a dumb naming thing in my head, but I think conceptually they’re two different things.)

I could theoretically use Caffeine for my "persistence" too. However, it’d be different instances than what I use to dedupe requests. At this point, I genuinely see no benefit in rewriting my code to use it over my "homemade maps" other than not manually removing expired entries myself.

SCjona commented 1 month ago

caches are for anything that is slow really. the usual things to cache are:

but yeah your current solution works, there is not much benefit of refactoring except maybe simpler code if you had actually used the LoadingCache callback system