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

Cache fs readFile operations #147

Closed
bringking opened this issue Feb 26, 2019 · 3 comments · Fixed by #149
Closed

Cache fs readFile operations #147

bringking opened this issue Feb 26, 2019 · 3 comments · Fixed by #149

Comments

@bringking
Copy link

bringking commented Feb 26, 2019

Feature Request

What

Thanks for the fantastic library. We are using this in production for a fairly high-traffic application. We are noticing in our APM that we are performing quite a few readFile operations. When diving in, we noticed they are mostly coming from Polyfill Library. This isn't causing a huge problem per-se, but it is introducing time into our request that I think can be reduced.

Details

Looking at the code here -
https://github.com/Financial-Times/polyfill-library/blob/master/lib/sources.js#L16

It seems we could replace this with a simple require call (which is sync, but cached). Or stick with the readFile approach and cache the result. This would obviously mean the files can't be updated after the app boots, but that seems like an edge case. Is this something you guys have thought about? I would be happy to submit a PR.

@JakeChampion
Copy link
Owner

Thanks for opening this issue @bringking. I think we would want to go with a least-recently-used-cache (LRU-cache) instead of the synchronous require. I'll make a pull-request for this work now :-)

@bringking
Copy link
Author

@JakeChampion I don't see that code change in the npm release, looks like the old code 🤔

@bringking
Copy link
Author

@JakeChampion any idea when you might fix the release? Would love to get this out

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 15, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants