Skip to content

Commit

Permalink
reduce memory consumption (#47)
Browse files Browse the repository at this point in the history
see : #37

The current implementation of reading files and creating polyfill
bundles doesn't really make sense.

It is both streaming and cached.
These are not things you can/should combine.

Either you make something streaming because you are concerned about
memory usage.
Or you have a cache and try to serve directly from memory.

So the current implementation is needlessly complex while also being
slow and using too much memory.

The slowness is mostly the result of having too many cache misses and an
implementation that is way too complex for what is actually going on.

------------

This change:
- make it a real streaming implementation
- reduce the number of filesystem operations by combining the
`meta.json` files for all polyfills and lazily loading this in memory
- reduce the number of async operations, especially waiting on things
that are actually sync reads
- pre-compute the dependency graph during the build process and store
the toposort order in `meta.json`
- remove as many needless conversions from streams to strings and back
- make the `fs` functions user configurable

------------

Configurable `fs` functions:

```js
/**
 * Set the file system implementation to use.
 * The default implementation uses the Node.js `fs` module.
 *
 * @param {readFile} readFile - A function that reads a file and returns a promise.
 * @param {createReadStream} createReadStream - A function that creates a read stream.
 */
function setFs(readFile, createReadStream) {
	fsImpl = {
		readFile,
		createReadStream
	};
}
```

This is a replacement for those users who want caching or other
optimizations.

I don't want to add an explicit cache mechanic as I still believe that
an in memory cache implemented in JavaScript is dangerous when running
this as a hosted service.

It is too easy to request different polyfills one by one, thrashing both
the `LRU` cache layer and the garbage collector to eventually starve the
host of both memory and CPU.

Either you want to have an edge cache in front of a hosted service
(making in memory cache less relevant) or you want to use something like
sqlite to optimize filesystem reads.

-------

I don't consider the removal of the `LRU` cache a breaking change
because this was an internal implementation detail. But I am open to
other opinions here :)
  • Loading branch information
romainmenke authored Oct 8, 2024
1 parent 48ad525 commit 17666c5
Show file tree
Hide file tree
Showing 21 changed files with 1,405 additions and 734 deletions.
3 changes: 3 additions & 0 deletions eslint.config.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,9 @@ export default [
"no-empty": ["error", {
"allowEmptyCatch": true,
}],
"no-unused-vars": ["error", {
"caughtErrors": "none",
}],
}
},
{
Expand Down
Loading

0 comments on commit 17666c5

Please sign in to comment.