[Feature] Add NPM package with API for Node.js

[JoakimCh]

I'm creating a web-server in Node.js and at first I used a minifier written in JavaScript, which as you can guess had a poor performance. Looking for something better I found this and esbuild, esbuild was easy to integrate into my project since it has an NPM package exposing a module which I can import into Node.js which contains an API to use it with Node.js.

All I have to do is to npm i esbuild and then for example:

let esbuild = require('esbuild')
let result1 = esbuild.transformSync(code, options)

I would rather use your minifier though, so it would be nice it that could be made possible in the future.

[tdewolff]

You're right, I should be looking to provide an NPM package. I'll find out how to do that, but if you have any info that would surely help!

[JoakimCh]

If you're new to the whole NPM and Node.js ecosystem then it can be a bit overwhelming to do this.

I guess the steps are something like this:

• Figure out a performant way for the Node.js module to communicate with the binary, maybe via Unix domain sockets. Or via stdin / stdout?

• Spawn the binary using Node.js "Child process" API.

• Research how package.json and publishing to NPM works.

• Have a NPM install script download the correct pre-compiled binary for the platform?

• Maybe consider writing an "ECMAScript module" instead of using the old "CommonJS" module system. If doing so then here is how to require CommonJS modules inside those newer modules (just bring back the require function):

import {createRequire} from 'module'
const require = createRequire(import.meta.url)

[JoakimCh]

I started thinking about how I would like the API to be like and came up with this.

// Example of a script using the module
import * as minify from '@tdewolff/minify' // Since there is already a "minify" NPM package it must be scoped

let result
result = await minify.file('/whatever.js')
result = await minify.content('js', 'let i=1; console.log(i)')

The module's main file could be something like this:

// The code here will run only once, even if the module is imported several places, hence you can connect to the binary here and the exported functions can communicate with it using any variables defined here outside of the functions.

export function file(path) {
  return new Promise((resolve, reject) => {
    // resolve(minifiedContentOfFile)
    // or reject('error message')
  })
}

export function content(typeOfContent, content) {

}

Of course these things are up to you :)

[tdewolff]

That looks pretty good to me, note the interface that esbuild has https://esbuild.github.io/api/#js-specific-details. Also see https://github.com/evanw/esbuild/tree/master/lib for a reference of the NPM package.

[privatenumber]

FYI esbuild adopted a new installation strategy in v0.13.0

Any updates on when this can be added? Would like to add your library to https://github.com/privatenumber/minification-benchmarks

[JoakimCh]

I'm actually almost done with an npm package with an API for minify and automatic download of the native binary. Will release soon.

[tdewolff]

Hi @JoakimCh that sound awesome! Thank you for the effort, I'd be happy to integrate it with this repository. Let me know if you need any help!

[JoakimCh]

Sorry about the late release, I'm struggling with some health problems and had to delay it, but here you guys go:
https://www.npmjs.com/package/tdewolff-minify

I didn't write any tests or documentation (other than JSDoc in the source-code) and I can't really be bothered with that for now... If this is production ready or not I don't have any opinions about, that's up to anyone to decide themselves.

Using this JS API might have some overhead compared to using Minify directly, hence performance tests shouldn't claim that they're testing the native performance of Minify.

const minify = new Minify({maxConcurrency: 4})
Initializing it like this will try to always run 4 concurrent processes when there's lots of files thrown at it.

const minify = new Minify()
This on the other hand will check how many threads your CPU has available and use up to that many processes.

All of these functions other than minify.fileToFile are using stdin and stdout to have the content minified, hence one minify process can only handle one file and has to be relaunched to handle the next file. Which probably adds the most overhead (waiting for it to be launched and ready again)... It might be interesting if one minify process could keep receiving files via stdin, maybe separated by something like 0x1C (ASCII code meaning "File Separator") and then use the same code in stdout.

[tdewolff]

@JoakimCh Excellent work! Thank you so much for taking the time, it is highly appreciated :-D

In terms of maintenance, do you wish to maintain the NPM port for the future, or would you like to incorporate it with this repository where you and I both can maintain it? Let me know what you think.

The case for streams, adding an ASCII code for file separation could possibly work, but otherwise we can make a special function for NPM access that for example accepts JS arrays of files or some JSON or binary structure (protocol buffers?). The idea of file separators could work efficiently (but I want to prevent scanning the file twice, first to find the file separators and then for parsing), though there might be an option to do that better (stop parsing at a file separator and start a new parser). Something like that, or by passing the file lengths of the concatenated files in a binary stream...

[JoakimCh]

I'm thinking that I can maintain it, since I plan to use your minifier for a server I'm developing. I'll look into how it can be merged with this repository though.

When it comes to performance. How does it handle the command minify -o out/ *? Are the files minified concurrently using several threads?

What I should do is to write a performance test which does the same, but using the JavaScript API. So we can analyze the performance difference and how to increase it.

Then if implementing different ways for JS to communicate with minify we will be able to measure them against each other.

[tdewolff]

Yes, the files are minified in parallel. There is no functionality yet to separate a single stream into different files, and simply concatenating is not possible. Am I correct that you're calling the minify binary and not the API of the Go library? I notice that you use streams to send data to the minifier binary, how slow is it to launch the binary? It would be nice to test how fast it is to minify file A twice and file B once, where B is double the size of file A. That is, we minify the same amount of data but the difference should be the slowdown due to launching the binary. See the benchmarks directory for some example files ;-)

In any case, it might be quite useful if the binary (not the library) would accept streams that separates files using the 0x1C file separator character. I'm going to work out that idea and come back to you ;-)

[JoakimCh]

Yes, it was easiest for me to use the precompiled minify binary. Since I'm not experienced when it comes to binding Node.js to functions in libraries.

I'm writing some tests now (when more complete I will merge them with my GitHub repository). But I wanted to share some of my findings already:

My API minify.fileToFile just launches a minify process that does all the work (no stdin/stdout used). And this didn't add much of an overhead when ran on each file in the benchmark folder compared to minify crunching all of them in one run.

There was some, but only if I forced only one concurrent minify process, with 4 concurrent ones there were no difference.

Part of my test code:

const dirents = fs.readdirSync('test_data/', {withFileTypes: true})
for (const dirent of dirents) {
  if (dirent.isFile()) {
    switch (2) {
      case 0: { // node reads the file and feeds it to minify, then node feeds stdout to a file
        const readStream = fs.createReadStream('test_data/'+dirent.name)
        const writeStream = fs.createWriteStream('nodeAPI_out/'+dirent.name)
        jobDonePromises.push(minify.pipe(extname(dirent.name).slice(1), readStream, writeStream))
      } break
      case 1: { // minify reads the file, node reads stdout
        jobDonePromises.push(minify.file('test_data/'+dirent.name))
      } break
      case 2: { // minify does file to file
        jobDonePromises.push(minify.fileToFile('test_data/'+dirent.name, 'nodeAPI_out/'+dirent.name))
      } break
    }
  }
}

case 0 was the slowest one, seemed to be 7 times slower
case 1 was around 4 times slower
case 2 was the one I mentioned above

JavaScript being single threaded is of course the biggest bottleneck I guess, so reading and writing files using a single thread is probably the problem...

I will try to learn some more though about what's going on.

[tdewolff]

Yes, a big part of the stream bottleneck will be JS performance. Case 2 has very little data that was send between JS and minify, since minify does all the file reading itself. What might be a great idea is that we could write one or more files to a temporary folder, run minify on that folder, then read the results back into JS. That would allow for compressing multiple files and might avoid the bottleneck of stdin/stdout streams between JS and the binary. Surely, the overhead is writing and reading files, so I'm not sure if it is worth it...

[JoakimCh]

I just uploaded the benchmark:
https://github.com/JoakimCh/tdewolff-minify/tree/main/tests/benchmark_versus_native
I can't be sure that using file separators will help performance, but if you want to test it then I can try writing an alternate API that will be optimized to use them.

[tdewolff]

Awesome work! I like how it automatically downloads the latest version from GitHub. I'm getting the following results:

Starting benchmarks...
Native minify: 555.978ms
minify.fileToFile: 709.665ms
minify.pipe: 4.091s
minify.file: 3.165s
minify.content: 3.325s
minify.content without file IO: 3.240s
All done.

The minify.content lines are strange. Are you sure that fs.readFileSync holds the content of the file, or is it just a file reading handle that will reread the file the next time?

I think we could add one additional test where the files are in-memory (read file content before the test) and test pipe of the file content and writing to a tmp file, using fileToFile, then reading from the tmp file. I like the latter one because we can easily scale that to minifying multiple files at once. I think the NPM port should provide two functions: content that may accept an array of contents, and file that minifies file to file on disk. What do you think?

[privatenumber]

Forgot to update:

I ended up using bina to install the binary.

tdewolff/minify is benchmarked on https://github.com/privatenumber/minification-benchmarks!

[tdewolff]

Nice, thanks for the addition! Obviously it seems that our interface for JS seems different than esbuild, because when compared as binaries, tdewolff/minify is a little bit faster.

[privatenumber]

The benchmarks should be fair and unbiased as possible. If there's a specific improvement you can recommend I'd be happy to welcome a PR or issue.

With the antd artifact (largest artifact), tdewolff/minify is slower thanesbuild by ~2.4s. tdewolff/minify is undoubtedly very fast but the JS interface cannot account for a 2.4s+ difference.

[tdewolff]

I think it does account for it. A quick test using time shows me that for me esbuild takes 0.587s while tdewolff/minify takes 0.428s (37% faster) on the antd artefact. I'll go ahead and see what is making the big difference for the JS port and come back to you ;-)

Appreciate the work a lot by the way!

[privatenumber]

I confirmed my data locally via binaries (no JS).

I cd into https://github.com/privatenumber/minification-benchmarks, made sure both minifiers are latest, and ran them with time and the hyperfine benchmarking tool (average of 10 runs).

I'm still getting a ~2.7s difference (3.227 s - 500.1 ms).

tdewolff/minify

$ node_modules/.bin/minify --version
minify v2.9.24

$ time node_modules/.bin/minify node_modules/antd/dist/antd.js > antd.tdewolff.js
node_modules/.bin/minify node_modules/antd/dist/antd.js > antd.tdewolff.js  0.85s user 2.24s system 101% cpu 3.045 total

$ hyperfine "node_modules/.bin/minify node_modules/antd/dist/antd.js > antd.tdewolff.js"              
Benchmark 1: node_modules/.bin/minify node_modules/antd/dist/antd.js > antd.tdewolff.js
  Time (mean ± σ):      3.227 s ±  0.374 s    [User: 0.858 s, System: 2.469 s]
  Range (min … max):    2.824 s …  4.028 s    10 runs

esbuild

$ node_modules/.bin/esbuild --version
0.14.8

$ time node_modules/.bin/esbuild node_modules/antd/dist/antd.js --minify > antd.esbuild.js
node_modules/.bin/esbuild node_modules/antd/dist/antd.js --minify >   0.35s user 0.08s system 98% cpu 0.433 total

$ hyperfine "time node_modules/.bin/esbuild node_modules/antd/dist/antd.js --minify > antd.esbuild.js"
Benchmark 1: time node_modules/.bin/esbuild node_modules/antd/dist/antd.js --minify > antd.esbuild.js
  Time (mean ± σ):     500.1 ms ±  71.0 ms    [User: 401.7 ms, System: 94.4 ms]
  Range (min … max):   433.2 ms … 629.2 ms    10 runs

Curious what specific commands you're running?

[tdewolff]

How strange, I run the same commands as you do, but I get 0.205s for tdewolff/minify and 0.407s for esbuild (I plugged in the power cable which makes it faster than the last time). I'm really struggling to understand that yours seems to be so extremely slow...what computer are you running on? I'm on an Intel i5-6300U @ 2.4GHz...

[privatenumber]

Local environment info:

$ npx envinfo --system

  System:
    OS: macOS 12.0.1
    CPU: (16) x64 Intel(R) Core(TM) i9-9880H CPU @ 2.30GHz
    Memory: 7.11 GB / 64.00 GB
    Shell: 5.8 - /bin/zsh

But for minification-benchmarks, benchmarking runs on GitHub Actions (which is yields the same results as my local env): https://github.com/privatenumber/minification-benchmarks/runs/4654820187?check_suite_focus=true

According to docs, it has:

• 2-core CPU
• 7 GB of RAM memory
• 14 GB of SSD disk space

Ubuntu 20.04.3 LTS:
https://github.com/actions/virtual-environments/blob/main/images/linux/Ubuntu2004-Readme.md

And what do you mean by the power cable that makes it faster?

(BTW, happy to take this to convo to a different Issue since this is a different topic)

[tdewolff]

Well, the power cable on my laptop enables higher performance ;-)

Could you please try using the --sequential and -o=out options for the binary (try the latest binary v2.9.25)? Perhaps this is a trivial problem of concurrency that doesn't work well for one file, or perhaps some buffering/streaming problem for stdout that we prevent by writing to a file. For esbuild you can use --outfile=out for the same thing. It is simply impossible that tdewolff/minify is not in the same ballpark, there must be something off.

[privatenumber]

Seems using -o to output the file made all the difference! Thank you.

$ hyperfine 'node_modules/.bin/minify -o=antd.tdewolff.js node_modules/antd/dist/antd.js' 'node_modules/.bin/esbuild node_modules/antd/dist/antd.js --outfile=antd.esbuild.js'
Benchmark 1: node_modules/.bin/minify -o=antd.tdewolff.js node_modules/antd/dist/antd.js
  Time (mean ± σ):     188.3 ms ±  13.0 ms    [User: 211.6 ms, System: 32.2 ms]
  Range (min … max):   168.5 ms … 215.6 ms    15 runs
 
Benchmark 2: node_modules/.bin/esbuild node_modules/antd/dist/antd.js --outfile=antd.esbuild.js
  Time (mean ± σ):     393.0 ms ±  17.8 ms    [User: 326.7 ms, System: 65.7 ms]
  Range (min … max):   369.1 ms … 422.0 ms    10 runs
 
Summary
  'node_modules/.bin/minify -o=antd.tdewolff.js node_modules/antd/dist/antd.js' ran
    2.09 ± 0.17 times faster than 'node_modules/.bin/esbuild node_modules/antd/dist/antd.js --outfile=antd.esbuild.js'

Curious if your environment didn't yield this performance difference between using pipe > and -o?

Wonder if there's anything that can be improved in the way tdewolff/minify pipes output because any JS API that wraps tdewolff/minify could be limited by this.

I will look into updating the minification benchmarks with this later but I might have to include the file-system read into the tdewolff/minify benchmark because this limitation will exist for any user that wants to use tdewolff/minify at peak performance.

I'm not sure if this will be a fair performance comparison because the speed of file-system read can vary and is not reflective purely of the minfier's performance while other minifiers provide a JS API that directly hands over the output via stdout.

[tdewolff]

Yes, surely I need to buffer the writing to stdout, perhaps this is automatic on my system but not on yours. Really unsure, I'm using bash, and you?

[privatenumber]

I'm using zsh. My env details provided in comment above:

$ npx envinfo --system

  System:
    OS: macOS 12.0.1
    CPU: (16) x64 Intel(R) Core(TM) i9-9880H CPU @ 2.30GHz
    Memory: 7.11 GB / 64.00 GB
    Shell: 5.8 - /bin/zsh

I ran benchmarks using bash but piping with tdewolff/minify is still significantly slower:

$ hyperfine \
> 'node_modules/.bin/esbuild node_modules/antd/dist/antd.js --outfile=antd.esbuild.js' \
> 'node_modules/.bin/esbuild node_modules/antd/dist/antd.js --minify > antd.esbuild.js' \
> 'node_modules/.bin/minify -o=antd.tdewolff.js node_modules/antd/dist/antd.js' \
> 'node_modules/.bin/minify node_modules/antd/dist/antd.js > antd.tdewolff.js'

Benchmark 1: node_modules/.bin/esbuild node_modules/antd/dist/antd.js --outfile=antd.esbuild.js
  Time (mean ± σ):     405.0 ms ±  23.7 ms    [User: 336.0 ms, System: 68.4 ms]
  Range (min … max):   375.2 ms … 437.9 ms    10 runs
 
Benchmark 2: node_modules/.bin/esbuild node_modules/antd/dist/antd.js --minify > antd.esbuild.js
  Time (mean ± σ):     350.8 ms ±  12.6 ms    [User: 289.5 ms, System: 57.1 ms]
  Range (min … max):   335.4 ms … 375.7 ms    10 runs
 
Benchmark 3: node_modules/.bin/minify -o=antd.tdewolff.js node_modules/antd/dist/antd.js
  Time (mean ± σ):     201.0 ms ±  66.3 ms    [User: 203.0 ms, System: 34.0 ms]
  Range (min … max):   170.7 ms … 388.4 ms    10 runs
 
  Warning: The first benchmarking run for this command was significantly slower than the rest (388.4 ms). This could be caused by (filesystem) caches that were not filled until after the first run. You should consider using the '--warmup' option to fill those caches before the actual benchmark. Alternatively, use the '--prepare' option to clear the caches before each timing run.
 
Benchmark 4: node_modules/.bin/minify node_modules/antd/dist/antd.js > antd.tdewolff.js
  Time (mean ± σ):      2.137 s ±  0.144 s    [User: 0.564 s, System: 1.621 s]
  Range (min … max):    2.021 s …  2.504 s    10 runs
 
Summary
  'node_modules/.bin/minify -o=antd.tdewolff.js node_modules/antd/dist/antd.js' ran
    1.75 ± 0.58 times faster than 'node_modules/.bin/esbuild node_modules/antd/dist/antd.js --minify > antd.esbuild.js'
    2.01 ± 0.68 times faster than 'node_modules/.bin/esbuild node_modules/antd/dist/antd.js --outfile=antd.esbuild.js'
   10.63 ± 3.58 times faster than 'node_modules/.bin/minify node_modules/antd/dist/antd.js > antd.tdewolff.js'

Also double checked without hyperfine and with/without time and I can confirm changing to bash doesn't affect the result.

In any case, I'm getting the same performance via GitHub Actions using ubuntu-latest so you should be able to use that as a testing environment.

Haven't tested but I think this workflow will suffice:

name: Benchmark tdewolff/minify

on:
  push:
    branches: [master]
  workflow_dispatch:

jobs:
  benchmark:
    runs-on: ubuntu-latest
    steps:
      - name: Benchmark tdewolff/minify
        run: |
          npm i antd
          npx bina tdewolff/minify
          time minify node_modules/antd/dist/antd.js > antd.tdewolff.js

[tdewolff]

Thanks, I've added the benchmark and added buffering for stdout. Should behave fine now in v2.9.26

[privatenumber]

Thanks!

I just updated https://github.com/privatenumber/minification-benchmarks and tdewolff/minify is the fastest minifier now 🎉

Shared the news here: https://twitter.com/privatenumbr/status/1476378923231547399

[tdewolff]

Great news! I have some other optimizations in mind that I'd like to look at. Meanwhile, I also want to see how to improve the gzip compression as it seems subpar compared to others. Thanks for the great work!

[perrin4869]

Oh, I tried rerunning the tests on latest ubuntu 22.04, and it passed the tests on node 16, but failed for node 18 and for node 14. I guess it's a matter of making builds for the remaining node versions and glibc versions, maybe should look into setting up github actions to do that. I can try to look into it this weekend too :)

[tdewolff]

Yes, we'd need to add support for various architectures and node versions and glibc, but isn't that getting a bit too many combinations? You think building from source could work on the remaining architectures?

GitHub actions sounds nice, can it compile on different architectures? Or can we force node-gyp to compile for another architecture?

Two ideas:

• It looks like we can set conditions in the binding.gyp. We could condition on OS and architecture and GLIBC etc., which point to different static library builds compiled by Go (i.e. the minify.a file setting GOOS etc)
• We can set --target_arch etc for node-gyp, I'm not sure if it works...

[perrin4869]

libxmljs2 that I linked above seems to have automated the process, maybe it'll be enough to do whatever it is they are doing?
There's also the Node-API which seems to not require different builds for different node versions?

[tdewolff]

Nice, it seems like we should be using the Node API instead of the V8 engine, I will work on moving the code towards that. Meanwhile, that document mentions using https://github.com/prebuild/prebuildify for building the various binaries for each architecture and upload them together with the NPM package (instead of uploading them in another location such as GitHub). This has my preference to avoid cluttering up the releases here on GitHub.

Anyways, what is the nan package for that you included in a PR?

[perrin4869]

Well, it contains a bunch of macros and helped in getting the current bindings work inside worker threads, but I guess it won't be needed anymore if we move to Node-API :)

[perrin4869]

Just tried the new implementation on the rollup integration and all tests passed like a charm!

[tdewolff]

Thanks for the feedback @perrin4869 ! Great to hear it works, does that mean you can build the bindings from source and it works for all Node versions? I suppose we still need to prebuild binaries (with https://github.com/prebuild/prebuildify) so that anyone can install it without needing Go being installed. Am I right?

[perrin4869]

I tried building it locally, then copying it into rollup-plugin-tdewolff-minify and running the tests, and it all passed. I only tested node 16 at this point, but would be suprised if you have problems running it in other node versions.
That's right, the only thing left is to integrate prebuildify there!
I guess the most challenging part here will be to package the different builds of minify.a, since those are built by go separately though...

[perrin4869]

The most peculiar thing that I didn't look into yet is that when closing a worker thread that has the minify module loaded, node will crash with a segfault... I noticed the same behavior both with nan and with node-api. At this point I don't know if this is an issue related to node or related to this module, but I'd like to dive into this deeper. Maybe I'll have some time during the weekend.

[tdewolff]

I've added integration with prebuildify that seems to work locally at version @tdewolff/[email protected] (which is not in-sync with the Go package, we'll fix that later). Can you confirm that it works for you? Do you have access to a non-Linux-x64 to see whether it builds when the prebuild binary is not available?

Now we need to incorporate the various platform we'd like to support. I've been looking at https://github.com/mafintosh/prebuildify-ci but I've also noticed that https://github.com/prebuild/prebuildify#options shows we can change the target architecture and platform. Perhaps we could just iterate over the architectures and for each build using Go and then build the prebuild?

[perrin4869]

Just tested!
Here are the results: dotcore64/rollup-plugin-tdewolff-minify#2
It is working perfectly locally on my PC (slackware64-current, node16), and on github actions, ubuntu 22.04 on node 14, 16, 18.
It is failing on older ubuntu (maybe need to build with an older version glibc?) and on macos...

[tdewolff]

Thanks, I've fixed the build-from-source, which should be invoked when the prebuild binary isn't available. In your case, for macos this should trigger a local build (make sure go is installed), can you check if this works?

[tdewolff]

It seems that the platform/arch options for prebuildify that are passed to node-gyp don't allow cross-platform compilation. I suppose we need some GitHub action or something else to really launch platform/arch combination and prebuild the binary. It looks like https://github.com/prebuild/prebuildify-cross doesn't support Windows. Not sure how to configure it so that it builds the binaries and publishes the package, but https://github.com/mafintosh/prebuildify-ci seems to upload them to GitHub and then downloads them. Can you help me with this? Do you have an idea what we could do?

I suggest we only support the darwin/win32/linux platforms and the x64 architecture (three binary builds).

[perrin4869]

Hey, I've been trying to get node-gyp to build on macos on this PR for a while now, but I cannot get it to build, and I don't have a mac to try this more easily locally either. Would be great if someone with a mac could figure out how to fix this, but from the error messages it sounds like a problem with the architecture settings...
I'll be happy to continue helping when this is fixed ><;;
Feel free to push to that branch if you got any ideas!

Edit: My coworker tried in his mac and got the same error...

[tdewolff]

It seems to be working for Linux and MacOS, and additionally I've build a bash script locally that downloads the artifacts generated by the workflow, so that I can package it and ship a new version for NodeJS. As this involves a manual step, happy to hear alternatives.

Secondly, I can't for the life get it to work on the Windows platform. I've fixed the initial issue, it compiles fine currently. But somehow it gives me weird error (see https://github.com/tdewolff/minify/runs/7119721849?check_suite_focus=true#step:4:85). The error suggests we've used a different node/npm version for building and for testing, but I don't see how...any help appreciated!

[perrin4869]

I've been experiencing a lot of segfaults running @tdewolff/minify in worker threads (for example, https://github.com/dotcore64/rollup-plugin-tdewolff-minify/runs/7076883743?check_suite_focus=true)
According to https://nodejs.org/dist/latest/docs/api/addons.html#worker-support,

In order to support [Worker](https://nodejs.org/dist/latest/docs/api/worker_threads.html#class-worker) threads, addons need to clean up any resources they may have allocated when such a thread exists. This can be achieved through the usage of the AddEnvironmentCleanupHook() function:

I think the root cause of the segfaults may be that

minify/bindings/js/minify.go

Line 21 in 6e2dbbd

var m *minify.M

is not being freed before the thread goes down.

Using the node-api, i think the way to fix it might be to call the go GC on https://nodejs.org/dist/latest/docs/api/n-api.html#napi_add_env_cleanup_hook after setting the minifier to nil, but i haven't had the chance to test this yet 😅

[tdewolff]

Thanks, I've instead opted for a way to keep the minifier allocated on the (global) stack, hopefully that resolves the issue. If it still doesn't, I suppose we need to call the GC explicitly.

[SalvatorePreviti]

Trying to install the latest version I get this error on Mac OS M1

npm i @tdewolff/minify --save-dev
npm ERR! code 1
npm ERR! path /Users/sp/my/xxx/node_modules/@tdewolff/minify
npm ERR! command failed
npm ERR! command sh -c node-gyp-build
npm ERR! ACTION Building Go library... minify.a
npm ERR! go build -buildmode=c-archive -o minify.a minify.go
npm ERR! CC(target) Release/obj.target/minify/minify.o
npm ERR! SOLINK(target) Release/minify.node
npm ERR! gyp info it worked if it ends with ok
npm ERR! gyp info using [email protected]
npm ERR! gyp info using [email protected] | darwin | arm64
npm ERR! gyp info find Python using Python version 3.9.12 found at "/opt/homebrew/opt/[email protected]/bin/python3.9"
npm ERR! gyp info spawn /opt/homebrew/opt/[email protected]/bin/python3.9
npm ERR! gyp info spawn args [
npm ERR! gyp info spawn args '/opt/homebrew/lib/node_modules/npm/node_modules/node-gyp/gyp/gyp_main.py',
npm ERR! gyp info spawn args 'binding.gyp',
npm ERR! gyp info spawn args '-f',
npm ERR! gyp info spawn args 'make',
npm ERR! gyp info spawn args '-I',
npm ERR! gyp info spawn args '/Users/sp/my/xxx/node_modules/@tdewolff/minify/build/config.gypi',
npm ERR! gyp info spawn args '-I',
npm ERR! gyp info spawn args '/opt/homebrew/lib/node_modules/npm/node_modules/node-gyp/addon.gypi',
npm ERR! gyp info spawn args '-I',
npm ERR! gyp info spawn args '/Users/sp/Library/Caches/node-gyp/16.15.1/include/node/common.gypi',
npm ERR! gyp info spawn args '-Dlibrary=shared_library',
npm ERR! gyp info spawn args '-Dvisibility=default',
npm ERR! gyp info spawn args '-Dnode_root_dir=/Users/sp/Library/Caches/node-gyp/16.15.1',
npm ERR! gyp info spawn args '-Dnode_gyp_dir=/opt/homebrew/lib/node_modules/npm/node_modules/node-gyp',
npm ERR! gyp info spawn args '-Dnode_lib_file=/Users/sp/Library/Caches/node-gyp/16.15.1/<(target_arch)/node.lib',
npm ERR! gyp info spawn args '-Dmodule_root_dir=/Users/sp/my/xxx/node_modules/@tdewolff/minify',
npm ERR! gyp info spawn args '-Dnode_engine=v8',
npm ERR! gyp info spawn args '--depth=.',
npm ERR! gyp info spawn args '--no-parallel',
npm ERR! gyp info spawn args '--generator-output',
npm ERR! gyp info spawn args 'build',
npm ERR! gyp info spawn args '-Goutput_dir=.'
npm ERR! gyp info spawn args ]
npm ERR! gyp info spawn make
npm ERR! gyp info spawn args [ 'BUILDTYPE=Release', '-C', 'build' ]
npm ERR! ../minify.c:138:12: warning: expression which evaluates to zero treated as a null pointer constant of type 'napi_value' (aka 'struct napi_value__ *') [-Wnon-literal-null-conversion]
npm ERR! return napi_ok;
npm ERR! ^~~~~~~
npm ERR! ../minify.c:246:12: warning: expression which evaluates to zero treated as a null pointer constant of type 'napi_value' (aka 'struct napi_value__ *') [-Wnon-literal-null-conversion]
npm ERR! return napi_ok;
npm ERR! ^~~~~~~
npm ERR! 2 warnings generated.
npm ERR! ld: warning: object file (../minify.a(000000.o)) was built for newer macOS version (12.0) than being linked (11.0)
npm ERR! ld: warning: object file (../minify.a(000004.o)) was built for newer macOS version (12.0) than being linked (11.0)
npm ERR! ld: warning: object file (../minify.a(000006.o)) was built for newer macOS version (12.0) than being linked (11.0)
npm ERR! ld: warning: object file (../minify.a(go.o)) was built for newer macOS version (12.0) than being linked (11.0)
npm ERR! ld: warning: object file (../minify.a(000015.o)) was built for newer macOS version (12.0) than being linked (11.0)
npm ERR! ld: warning: object file (../minify.a(000009.o)) was built for newer macOS version (12.0) than being linked (11.0)
npm ERR! ld: warning: object file (../minify.a(000005.o)) was built for newer macOS version (12.0) than being linked (11.0)
npm ERR! ld: warning: object file (../minify.a(000007.o)) was built for newer macOS version (12.0) than being linked (11.0)
npm ERR! ld: warning: object file (../minify.a(000010.o)) was built for newer macOS version (12.0) than being linked (11.0)
npm ERR! Undefined symbols for architecture arm64:
npm ERR! "_CFStringCreateWithBytes", referenced from:
npm ERR! _crypto/x509/internal/macos.x509_CFStringCreateWithBytes_trampoline.abi0 in minify.a(go.o)
npm ERR! "_napi_coerce_to_string", referenced from:
npm ERR! _config in minify.o
npm ERR! "_napi_create_function", referenced from:
npm ERR! _init in minify.o
npm ERR! "_napi_create_string_utf8", referenced from:
npm ERR! _string in minify.o
npm ERR! "_napi_get_all_property_names", referenced from:
npm ERR! _config in minify.o
npm ERR! "_napi_get_array_length", referenced from:
npm ERR! _config in minify.o
npm ERR! "_napi_get_cb_info", referenced from:
npm ERR! _config in minify.o
npm ERR! _string in minify.o
npm ERR! _file in minify.o
npm ERR! "_napi_get_element", referenced from:
npm ERR! _config in minify.o
npm ERR! "_napi_get_property", referenced from:
npm ERR! _config in minify.o
npm ERR! "_napi_get_value_string_utf8", referenced from:
npm ERR! _get_string in minify.o
npm ERR! "_napi_module_register", referenced from:
npm ERR! __register_minify in minify.o
npm ERR! "_napi_set_named_property", referenced from:
npm ERR! _init in minify.o
npm ERR! "_napi_throw_error", referenced from:
npm ERR! _config in minify.o
npm ERR! _string in minify.o
npm ERR! _file in minify.o
npm ERR! "_napi_throw_type_error", referenced from:
npm ERR! _config in minify.o
npm ERR! _string in minify.o
npm ERR! _file in minify.o
npm ERR! "_napi_typeof", referenced from:
npm ERR! _get_string in minify.o
npm ERR! _config in minify.o
npm ERR! ld: symbol(s) not found for architecture arm64
npm ERR! clang: error: linker command failed with exit code 1 (use -v to see invocation)
npm ERR! make: *** [Release/minify.node] Error 1
npm ERR! gyp ERR! build error
npm ERR! gyp ERR! stack Error: make failed with exit code: 2
npm ERR! gyp ERR! stack at ChildProcess.onExit (/opt/homebrew/lib/node_modules/npm/node_modules/node-gyp/lib/build.js:194:23)
npm ERR! gyp ERR! stack at ChildProcess.emit (node:events:527:28)
npm ERR! gyp ERR! stack at Process.ChildProcess._handle.onexit (node:internal/child_process:291:12)
npm ERR! gyp ERR! System Darwin 21.4.0
npm ERR! gyp ERR! command "/usr/local/bin/node" "/opt/homebrew/lib/node_modules/npm/node_modules/node-gyp/bin/node-gyp.js" "rebuild"
npm ERR! gyp ERR! cwd /Users/sp/my/xxx/node_modules/@tdewolff/minify
npm ERR! gyp ERR! node -v v16.15.1
npm ERR! gyp ERR! node-gyp -v v9.0.0
npm ERR! gyp ERR! not ok

npm ERR! A complete log of this run can be found in:
npm ERR! /Users/sp/.npm/_logs/2022-07-06T11_37_57_230Z-debug-0.log

[tdewolff]

I've updated the release on NPM manually, but this will happen automatically in the future using GitHub workflows (including prebuild binaries).

[tdewolff]

The NodeJS workflow is now running for all three targets, see https://github.com/tdewolff/minify/actions/runs/2626161819

I will try and figure out the sigfault now.

[perrin4869]

@tdewolff Thank you so much for this! I think it's closed now :)

[perrin4869]

Ah no, the os.Exit is terminating the whole node process: https://github.com/dotcore64/rollup-plugin-tdewolff-minify/runs/7272307346?check_suite_focus=true

It is exiting after the first test...

[tdewolff]

Hm, this must be a problem between Go and Node and I'm not sure how to fix this. It's at least not a memory issue with this library, but something to do with the garbage collector or a race condition. I've tried running Go on a locked OS thread, by calling GC on clean up etc but nothing works. The segfault also happens when no use code is executed in Go (i.e. empty function). I'm pretty sure we'd need to take this up to Go or node-gyp/NAPI. Meanwhile, some other native addons actually have the same problem I've noticed (the node-ffi package notably).

Anyways, v.2.12.0 has been released with the JS binaries included, so this issue can be considered resolved.

[tdewolff]

@privatenumber This might also speed up the minifier in your benchmarks, you think you could give it a try? Package is available from https://www.npmjs.com/package/@tdewolff/minify