Use deno 1.45: binaries ~25% smaller, ~10% faster

[felipecrs]

Note: this description is outdated.

This PR builds on #959, and bumps deno to 1.41.

With this change, the pkgx binaries size were decrease by 40MB in Linux AMD64:

PS: you can close this PR. I just wanted to demonstrate the benefits of upgrading to deno 1.41.

You guys know best what to do. Probably some changes in libpkgx are needed as well.

Closes #959

[mxcl]

Yeah totes want it. We’re having trouble building 1.41 over at the pantry, and couldn't bump to 1.40 due to deprecated API with no replacements at that time. Once 1.41 builds we can figure it all out.

[felipecrs]

Awesome!

[mxcl]

FYI if you strip deno first it results in smaller binaries too.

[felipecrs]

Right, which can be done even with deno 1.39. But I don't think is so urgent. :)

[jhheider]

1.41 likely in an hour or so. or two. it's not exactly fast to build.

[jhheider]

1.41.0 not ready for linux/aarch64. I pushed fixes for the other 3, but it uses their build of denort, which uses ubuntu22's glibc. You can see the outstanding issue as a comment in the package.yml.

[felipecrs]

Right, and apparently, they don't allow to specify your own denort binary as well. Thanks for following it up.

[mxcl]

So main remaining issue is libpkgx uses Deno.run which is deprecated but is necessary to build libpkgx with dnt so we can publish to npm because Deno.Command is not available to the dnt shims.

And deno does not make it possible to silence the shouted warning in compiled binaries about the deprecated API usage.

At least that was true with deno 1.40

[felipecrs]

And deno does not make it possible to silence the shouted warning in compiled binaries about the deprecated API usage.

The binary I compiled is not throwing any warnings when invoked. Is there some specific command to trigger it?

Code_SbayEe6Xw8.mp4

[felipecrs]

@mxcl I made some additional tries and I cannot trigger any deprecation warning. Maybe this is good to go?

but it uses their build of denort, which uses ubuntu22's glibc

@jhheider, is this a no-go?

[jhheider]

it is for building the same deno across all environments for the time being. per their notes in the PR:

    # https://github.com/denoland/deno/pull/22298
    # deno.land 1.41.0 will currently _not_ run deno compile on linux/aarch64
    # for their first official release, they're using ubuntu 22.04, which means
    # a newer glibc. Patching via the https://github.com/LukeChannings/deno-arm64
    # repo may be possible, but lets not delay the three usable arches for the rare
    # one. Revist this.

the problem is, deno downloads denort, rather than using the local one; so we'd need to hack their code to make it first check for a local denort. that's potentially possible, but i haven't dug into it. i assume our compiled denort will still work, so fixing that could be the easiest path forward.

otherwise, we could put in build logic that uses newer deno for 3/4 of our platforms, as long as it's drop-in and isn't diverging from the old one too rapidly. that'd reduce binary sizes for our most-used platforms.

[felipecrs]

Ok. I see now. Here's a quick reproduction of the problem btw (I used it to verify whether 1.41.2 had solved it or not -- it didn't).

FROM denoland/deno:1.41.2 AS build

RUN apt-get update && apt-get install unzip -y

RUN deno compile https://docs.deno.com/examples/welcome.ts

FROM ubuntu:18.04

COPY --from=build /welcome /

ADD "https://www.random.org/cgi-bin/randbyte?nbytes=10&format=h" skipcache
RUN /welcome

# install arm64 emulator
$ docker run --privileged --rm --pull=always tonistiigi/binfmt --install arm64

$ docker build . --platform=amd64
[...]
Welcome to Deno!

$ docker build . --platform=arm64
[...]
/welcome: /lib/aarch64-linux-gnu/libm.so.6: version `GLIBC_2.29' not found (required by /welcome)

I'll let you know if I find out something.

[felipecrs]

i assume our compiled denort will still work, so fixing that could be the easiest path forward.

Which "our" compiled denort? It's not in the pantry from what I can tell.

That's the easiest path forward I can think of as well. And I don't think it should be considered a workaround either, as I think it would be desirable to have control over the libs used to compile denort and therefore pkgx itself.

I think what it would take is:

1. pkg denort in pantry
2. apply a patch to pkgx's deno, it should download denort from the pantry rather than downloading it from the official deno binaries

The best possible solution IMO would be if deno could self-package the denort in runtime (after all it's a sub-set of the full deno binary).

[jhheider]

current builds of deno.land also produce bin/denort (via cargo install). So, if we could first check the path, or alongside the deno binary, we'd be in good shape.

[felipecrs]

Understood! Will look into it. Thanks.

[felipecrs]

Checking the denort binary beside deno may not be the best option because deno is capable of cross compiling it too (by downloading the extra denort binaries).

[felipecrs]

Checking the denort binary beside deno may not be the best option because deno is capable of cross compiling it too (by downloading the extra denort binaries).

But it's ok in pkgx's case because cross compilation is not used.

@jhheider take a look at my latest commit, I think it's good enough now. :D

f358c14

https://github.com/denoland/deno/blob/5d671e079a3aada2fd1efccd82a3fe4fd33c512e/cli/standalone/binary.rs#L484

The bad news however is:

❯ ls -lah pkgx*
-rwxrwxrwx 1 felipecrs felipecrs  88M Mar  8 14:45 pkgx.deno-denort
-rwxrwxrwx 1 felipecrs felipecrs 106M Mar  8 14:44 pkgx.pkgx-denort

But it's an improvement anyway over the previous 118MB. We can probably trim this size down by compiling denort in the pipeline with some different flags. We can potentially make the final denort size even smaller than deno's own denort.

[jhheider]

Good find! @mxcl what do you think? I'm hesitant to runtime.env this, since it'll break cross compiling, but this should fix our builds.

[felipecrs]

@mxcl I also fixed some flaky execve tests on Ubuntu and macOS. On Ubuntu, an actual fix was done. On macOS, it's more like improving the previous workaround.

[mxcl]

This is good, but will not merge with the unnecessary prettier changes to the YAML. I probs will just fix the PR myself or squash and then fix.

edit: specifically the fixes to the execve.ts code would never have occurred to me and fix long standing test failure issues. Superb!

[felipecrs]

@mxcl, let me fix the prettier stuff, one sec.

[coveralls]

Pull Request Test Coverage Report for Build 10739559005

Details

• 17 of 19 (89.47%) changed or added relevant lines in 2 files are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage decreased (-0.2%) to 93.807%

---

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
src/utils/execve.ts	13	15	86.67%

Totals
Change from base Build 10727606337:	-0.2%
Covered Lines:	1453
Relevant Lines:	1550

---

💛 - Coveralls

[felipecrs]

Please hold on. It seems that 1.46 has regressed in performance.

Summary
  ./pkgx.1.45 [email protected] --version ran
    1.05 ± 0.04 times faster than /home/felipecrs/.local/bin/pkgx [email protected] --version
    1.06 ± 0.05 times faster than ./pkgx.1.44 [email protected] --version
    1.07 ± 0.03 times faster than ./pkgx.1.43 [email protected] --version
    1.12 ± 0.04 times faster than ./pkgx.1.46 [email protected] --version

[felipecrs]

Ok, 1.46 is the least performant version of deno by far, while 1.45 is the best one. I am reverting to it for now.

❯ hyperfine --warmup 1 --shell=none './pkgx.1.43 [email protected] --version' './pkgx.1.44 [email protected] --version' './pkgx.1.45 [email protected] --version' './pkgx.1.46 [email protected] --version' --runs 50
Benchmark 1: ./pkgx.1.43 [email protected] --version
  Time (mean ± σ):     558.9 ms ±   4.8 ms    [User: 506.3 ms, System: 255.1 ms]
  Range (min … max):   550.7 ms … 574.1 ms    50 runs

Benchmark 2: ./pkgx.1.44 [email protected] --version
  Time (mean ± σ):     538.7 ms ±   4.0 ms    [User: 527.6 ms, System: 247.4 ms]
  Range (min … max):   528.3 ms … 545.8 ms    50 runs

Benchmark 3: ./pkgx.1.45 [email protected] --version
  Time (mean ± σ):     512.5 ms ±   3.9 ms    [User: 458.6 ms, System: 247.4 ms]
  Range (min … max):   503.7 ms … 520.1 ms    50 runs

Benchmark 4: ./pkgx.1.46 [email protected] --version
  Time (mean ± σ):     577.0 ms ±   4.0 ms    [User: 478.9 ms, System: 293.5 ms]
  Range (min … max):   570.1 ms … 584.6 ms    50 runs

Summary
  ./pkgx.1.45 [email protected] --version ran
    1.05 ± 0.01 times faster than ./pkgx.1.44 [email protected] --version
    1.09 ± 0.01 times faster than ./pkgx.1.43 [email protected] --version
    1.13 ± 0.01 times faster than ./pkgx.1.46 [email protected] --version

@jhheider, it is NOT a fault introduced by packaging, since I compared with the official deno binary:

❯ hyperfine --warmup 1 --shell=none './pkgx.1.46 [email protected] --version' './pkgx.1.46-nopkgx [email protected] --version' --runs 50
Benchmark 1: ./pkgx.1.46 [email protected] --version
  Time (mean ± σ):     580.4 ms ±   6.1 ms    [User: 486.9 ms, System: 291.2 ms]
  Range (min … max):   567.0 ms … 595.1 ms    50 runs

Benchmark 2: ./pkgx.1.46-nopkgx [email protected] --version
  Time (mean ± σ):     579.9 ms ±   7.6 ms    [User: 483.2 ms, System: 293.2 ms]
  Range (min … max):   566.1 ms … 610.2 ms    50 runs

Summary
  ./pkgx.1.46-nopkgx [email protected] --version ran
    1.00 ± 0.02 times faster than ./pkgx.1.46 [email protected] --version

Which is slightly faster because it is built against a newer version of libc, therefore less portable, if I remember well.

[mxcl]

Many thanks! Sorry for the ridiculous delay. Things should be more aligned for me now.

[felipecrs]

@mxcl had you seen this comment?

I noticed you upgraded Deno to 1.46.

[mxcl]

oof, ok, let’s go with ~1.45