revert(perf): drop the dockerode and docker-modem pnpm patches
Both patches came out of this work and both are removed at the maintainer's request. The finding was real - ssh2's WASM heap costs +14.0 MiB RSS on require, and @grpc/grpc-js another 14.6 MiB across 61 files, neither of which a socket-based Docker install ever uses - but patching a dependency means every future dockerode or docker-modem bump fails until someone regenerates the patch, and that recurring cost is not worth roughly 25 MiB. The finding stays on record in SERVER-MEMORY.md because the fix belongs upstream: both packages require a module at file scope while using it in exactly one already-guarded branch, so a PR to each would fix it for every consumer with no patch to maintain here. The two remaining patches, @types/node-unifi and trpc-to-openapi, predate this work and are untouched. allowUnusedPatches was already set on dev and is left alone. Lockfile regenerated: patchedDependencies now lists only those two. Also sharpens the Cache Components entry. The SqliteError was simply an unmigrated build database, which is fixable - but feeding the build a database would be the wrong fix, because serverSetting is per-installation data on the user's volume and a shell prerendered against the build machine's database would bake one install's settings into every other install's first paint. The error was Next.js correctly reporting a request-time read sitting in the shell path.
This commit is contained in:
8 files changed
+42
-72
No files matched your search
@@ -94,7 +94,7 @@ lines-of-code to performance ratio. On the evidence, that set is:
|
||||
| Umami: stop fetching 100k raw events per refresh | 1 file | −26% peak, −93% transient buffers, largest allocation 46.4 → 2.4 MiB |
|
||||
| `serverExternalPackages` (mysql2/pg + 8 more) | 1 block | −21 MiB idle |
|
||||
| `content-visibility` on media-release cards | 4 lines | −59% image requests, −41% layout objects |
|
||||
| ssh2 WASM heap (docker-modem patch) | 1 patch | −15 to −18 MiB external at every stage |
|
||||
| ~~ssh2 WASM heap (docker-modem patch)~~ | ~~1 patch~~ | **dropped** — the gain was real (−15 to −18 MiB external) but patching a dependency breaks on every `dockerode`/`docker-modem` bump. Worth raising upstream instead: both packages require a module at file scope while using it in one already-guarded branch |
|
||||
| Reuse the certificate agent instead of one per request | 1 file | 5.11 → 1.69 ms/request, 60 → 2 TLS handshakes over 60 requests |
|
||||
| Stream proxied images instead of buffering | 2 files | −36% peak RSS on 30 concurrent 2 MiB images (116 → 74 MiB) |
|
||||
|
||||
|
||||
+6
-1
@@ -37,9 +37,14 @@ No before/after range overlaps at any stage. The cost is **+18% CPU** (solid, no
|
||||
| `preloadEntriesOnStart: false` | **1 line** | −52% anon at idle, 310 fewer files even at full load |
|
||||
| Umami: stop fetching 100k raw events per refresh | 1 file | −26% peak, −93% transient buffers |
|
||||
| `serverExternalPackages` (mysql2/pg + 8 more) | 1 block | −21 MiB idle |
|
||||
| ssh2's 16 MiB WASM heap (docker-modem patch) | 1 patch | −15 to −18 MiB external |
|
||||
| Reuse the certificate agent instead of one per request | 1 file | 5.11 → 1.69 ms/request, 60 → 2 TLS handshakes |
|
||||
| Stream proxied images instead of buffering them | 2 files | −36% peak RSS under 30 concurrent 2 MiB images |
|
||||
| `content-visibility` on media-release cards | 4 lines | −59% image requests, −41% layout objects |
|
||||
|
||||
The ssh2 / `@grpc/grpc-js` finding (−15 to −18 MiB external) is deliberately **not** in this table: it
|
||||
was implemented as two pnpm patches and then reverted, because patching a dependency breaks on every
|
||||
upstream bump. The finding is recorded in [SERVER-MEMORY.md](./SERVER-MEMORY.md) and belongs upstream.
|
||||
|
||||
## What is left, and why it is hard
|
||||
|
||||
One lever has real headroom and it pays twice. Roughly **41 MiB of retained bundle source** and
|
||||
|
||||
+15
-6
@@ -31,12 +31,21 @@ const [session, user, serverSettings, colorScheme] = await Promise.all([
|
||||
]);
|
||||
```
|
||||
|
||||
For a self-hosted app whose database is created at runtime on the user's volume, there is nothing to
|
||||
query during a build. Fixing it means deferring those reads into `<Suspense>` — but all four feed
|
||||
providers that wrap `children` (`AuthProvider session=`, `SettingsProvider user=`), so deferring them
|
||||
puts essentially the whole app inside the boundary and leaves a static shell of little more than
|
||||
`<html><body>`. The build would report `◐` while shipping an empty shell, which is precisely the
|
||||
failure the migration guide warns about.
|
||||
The immediate cause is mundane: no migration had been run, so the build had no tables. That is fixable
|
||||
— run the migration first and the build gets a database.
|
||||
|
||||
**But making the error go away that way would be the wrong fix, and the error is the useful signal.**
|
||||
If a build needs the database in order to prerender a route, the route's content is being decided at
|
||||
build time rather than per request. For Homarr that is actively wrong: `serverSetting` is
|
||||
per-installation data on the user's own volume, so a shell prerendered against the build machine's
|
||||
database would bake one install's settings into every other install's first paint. The error is Next.js
|
||||
correctly reporting that a request-time read sits in the shell path.
|
||||
|
||||
The honest fix is therefore to defer those reads into `<Suspense>`, not to feed the build a database.
|
||||
And that is where it stops being worth it: all four values feed providers that wrap `children`
|
||||
(`AuthProvider session=`, `SettingsProvider user=`), so deferring them puts essentially the whole app
|
||||
inside the boundary and leaves a static shell of little more than `<html><body>`. The build would
|
||||
report `◐` while shipping an empty shell — precisely the failure the migration guide warns about.
|
||||
|
||||
**And the prize is small.** Measured against a running container, warm, five repeats:
|
||||
|
||||
|
||||
@@ -456,9 +456,21 @@ Measured directly inside the image — requiring ssh2 and nothing else:
|
||||
heapUsed +2.6 MiB external +0.9 MiB rss +14.0 MiB
|
||||
```
|
||||
|
||||
Fixed with a pnpm patch moving that one require inside the branch that uses it. SSH-based
|
||||
Docker hosts still work — `require` is cached, so they pay it once — and socket-based installs,
|
||||
which is everyone else, never load it.
|
||||
This was fixed with a pnpm patch moving that one require inside the branch that uses it, and the
|
||||
patch has since been **removed** — see below.
|
||||
|
||||
> **Reverted, by decision.** Two pnpm patches came out of this work: `docker-modem@5.0.7` (the ssh2
|
||||
> require above) and `dockerode@5.0.1` (the same shape, hiding `@grpc/grpc-js` — 14.6 MiB across 61
|
||||
> files — behind `dialWithSession`). Both were dropped at the maintainer's request: patching a
|
||||
> dependency means every future `dockerode` or `docker-modem` bump fails until someone regenerates the
|
||||
> patch, and that recurring cost was judged not worth roughly 25 MiB of RSS.
|
||||
>
|
||||
> The finding itself stands and is worth keeping on record, because the fix belongs **upstream**: both
|
||||
> packages require a module at file scope while using it in exactly one already-guarded branch. A PR to
|
||||
> `docker-modem` and `dockerode` would fix it for every consumer, with no patch to maintain here.
|
||||
>
|
||||
> The two patches that remain — `@types/node-unifi` and `trpc-to-openapi` — predate this work and are
|
||||
> untouched.
|
||||
|
||||
### The same question asked of every dependency
|
||||
|
||||
|
||||
@@ -1,27 +0,0 @@
|
||||
diff --git a/lib/modem.js b/lib/modem.js
|
||||
index b45a798d63647d55e0193fd7e82c2e36785d9b5f..d2dbf42969fc0ebf39b30c1a27373c33c2d20333 100644
|
||||
--- a/lib/modem.js
|
||||
+++ b/lib/modem.js
|
||||
@@ -3,7 +3,6 @@ var querystring = require('querystring'),
|
||||
fs = require('fs'),
|
||||
path = require('path'),
|
||||
url = require('url'),
|
||||
- ssh = require('./ssh'),
|
||||
HttpDuplex = require('./http_duplex'),
|
||||
debug = require('debug')('modem'),
|
||||
utils = require('./utils'),
|
||||
@@ -263,8 +262,13 @@ Modem.prototype.buildRequest = function (options, context, data, callback) {
|
||||
var connectionTimeoutTimer;
|
||||
var finished = false;
|
||||
|
||||
+ // Loaded here rather than at module scope: './ssh' pulls in ssh2, whose crypto module
|
||||
+ // instantiates a ~16 MiB WebAssembly memory for ChaCha20-Poly1305 as soon as it is
|
||||
+ // required. Measured at +14 MiB RSS for a require alone, paid by every Homarr install
|
||||
+ // even though Docker is reached over a socket and this branch never runs. require() is
|
||||
+ // cached, so SSH connections pay it once and behave exactly as before.
|
||||
var opts = self.protocol === 'ssh' ? Object.assign(options, {
|
||||
- agent: ssh(Object.assign({}, self.sshOptions, {
|
||||
+ agent: require('./ssh')(Object.assign({}, self.sshOptions, {
|
||||
'host': self.host,
|
||||
'port': self.port,
|
||||
'username': self.username,
|
||||
@@ -1,25 +0,0 @@
|
||||
diff --git a/lib/docker.js b/lib/docker.js
|
||||
index 4951f8db867b1b12b75af79be2ebae6165534960..b333e65e99a9ef420826c0b1a908d4923dffc2f6 100644
|
||||
--- a/lib/docker.js
|
||||
+++ b/lib/docker.js
|
||||
@@ -12,7 +12,6 @@ var EventEmitter = require('events').EventEmitter,
|
||||
Node = require('./node'),
|
||||
Exec = require('./exec'),
|
||||
util = require('./util'),
|
||||
- withSession = require('./session'),
|
||||
extend = util.extend;
|
||||
|
||||
var Docker = function(opts) {
|
||||
@@ -301,6 +300,12 @@ Docker.prototype.buildImage = function(file, opts, callback) {
|
||||
|
||||
function dialWithSession(callback) {
|
||||
if (opts?.version === "2") {
|
||||
+ // Loaded here rather than at module scope: './session' pulls in @grpc/grpc-js and
|
||||
+ // @grpc/proto-loader (112 files), which cost +6.0 MiB RSS on top of dockerode itself.
|
||||
+ // Only the BuildKit image-build path reaches this branch, so listing or starting
|
||||
+ // containers paid for a gRPC stack it never used. require() is cached, so builds behave
|
||||
+ // exactly as before.
|
||||
+ var withSession = require('./session');
|
||||
withSession(self, optsf.authconfig,(err, sessionId, done) => {
|
||||
if (err) {
|
||||
return callback(err);
|
||||
Generated
+5
-7
@@ -585,8 +585,6 @@ overrides:
|
||||
|
||||
patchedDependencies:
|
||||
'@types/node-unifi': 5e6ae51e2a17a7f9729bfa30b0eb3d0842a5810ac6db47603ab4a6efa1ed84c5
|
||||
docker-modem@5.0.7: 4a1c50dc8b726af921a34a77c4942a768bed229123ab116baa06c4e62ad6815c
|
||||
dockerode@5.0.1: 0f6e47505aa4a9aa6fabe6a42b070ca984c787d197bcac3aa6367961f2f7eac3
|
||||
trpc-to-openapi: 2ca3c16af0fcca0c736697ad4fe553a14f794524fa9ce0d5c3e8ee4aea76090c
|
||||
|
||||
importers:
|
||||
@@ -1782,7 +1780,7 @@ importers:
|
||||
version: link:../core
|
||||
dockerode:
|
||||
specifier: 'catalog:'
|
||||
version: 5.0.1(patch_hash=0f6e47505aa4a9aa6fabe6a42b070ca984c787d197bcac3aa6367961f2f7eac3)(supports-color@10.2.2)
|
||||
version: 5.0.1(supports-color@10.2.2)
|
||||
devDependencies:
|
||||
'@homarr/tsconfig':
|
||||
specifier: workspace:^0.1.0
|
||||
@@ -23648,7 +23646,7 @@ snapshots:
|
||||
dependencies:
|
||||
yaml: 2.9.0
|
||||
|
||||
docker-modem@5.0.7(patch_hash=4a1c50dc8b726af921a34a77c4942a768bed229123ab116baa06c4e62ad6815c)(supports-color@10.2.2):
|
||||
docker-modem@5.0.7(supports-color@10.2.2):
|
||||
dependencies:
|
||||
debug: 4.4.3(supports-color@10.2.2)
|
||||
readable-stream: 3.6.2
|
||||
@@ -23657,12 +23655,12 @@ snapshots:
|
||||
transitivePeerDependencies:
|
||||
- supports-color
|
||||
|
||||
dockerode@5.0.1(patch_hash=0f6e47505aa4a9aa6fabe6a42b070ca984c787d197bcac3aa6367961f2f7eac3)(supports-color@10.2.2):
|
||||
dockerode@5.0.1(supports-color@10.2.2):
|
||||
dependencies:
|
||||
'@balena/dockerignore': 1.0.2
|
||||
'@grpc/grpc-js': 1.12.5
|
||||
'@grpc/proto-loader': 0.7.13
|
||||
docker-modem: 5.0.7(patch_hash=4a1c50dc8b726af921a34a77c4942a768bed229123ab116baa06c4e62ad6815c)(supports-color@10.2.2)
|
||||
docker-modem: 5.0.7(supports-color@10.2.2)
|
||||
protobufjs: 7.4.0
|
||||
tar-fs: 2.1.4
|
||||
transitivePeerDependencies:
|
||||
@@ -29220,7 +29218,7 @@ snapshots:
|
||||
byline: 5.0.0
|
||||
debug: 4.4.3(supports-color@10.2.2)
|
||||
docker-compose: 1.4.2
|
||||
dockerode: 5.0.1(patch_hash=0f6e47505aa4a9aa6fabe6a42b070ca984c787d197bcac3aa6367961f2f7eac3)(supports-color@10.2.2)
|
||||
dockerode: 5.0.1(supports-color@10.2.2)
|
||||
get-port: 5.1.1
|
||||
proper-lockfile: 4.1.2
|
||||
properties-reader: 3.0.1(supports-color@10.2.2)
|
||||
|
||||
@@ -233,8 +233,6 @@ overrides:
|
||||
"vite@>=5.0.0 <=5.4.18": ">=8.1.4"
|
||||
patchedDependencies:
|
||||
"@types/node-unifi": "patches/@types__node-unifi.patch"
|
||||
docker-modem@5.0.7: patches/docker-modem@5.0.7.patch
|
||||
dockerode@5.0.1: patches/dockerode@5.0.1.patch
|
||||
"trpc-to-openapi": "patches/trpc-to-openapi.patch"
|
||||
|
||||
allowUnusedPatches: true
|
||||
|
||||
Reference in new issue
Block a user