@ChrisPenner This PR upgrades us to GHC 9.10 … and I think the compiler must be doing something differently somewhere, because the optchecks stuff is failing with effectively no change to unison-runtime. Maybe it just needs more of that throwIO workaround or something, but thought you might have a clue to offer.

unison-runtime:optchecks is failing, and I’m not sure why. Would love some assistance with this one.

@dolio If you might have an idea on this; possibly due to ghc version change?

That test checks that GHC is generating code that looks properly optimized. I think it basically checks that the eval function contains no mention of the Stack type.

So, the details of the optimization have changed. I guess you could try running benchmarks before and after to see if the difference actually matters. If it doesn't, then maybe we need a different test. But if it does, we need to figure out how to fix the performance. Even if it doesn't, it might be easier to tweak something to satisfy the check than to figure out a new check that's actually proper.

Here’s the Nix workflow run on my fork: https://github.com/sellout/unison/actions/runs/20785608716 (it’ll run on trunk in this repo when this PR is merged).

Some tests with 'continue-on-error: true' have failed:

• Cabal / smoke-test

Created by continue-on-error-comment

I think that the docs for building with cabal need to either switch back to project-file or need to otherwise give a working command (perhaps moving cabal files out of contrib or suggest adding a symlink). Otherwise this is working well for me.

I removed the commit with the Cabal doc changes. I prefer the shorter --project-dir=contrib over --project-file=contrib/cabal.project, and I like the paths to be relative to the file they’re in … but it breaks the ln -s contrib/* ./ workaround, which is nice for people who are using Cabal regularly.

NB: An annoying thing here is that because of the way the Cabal workflow works (in order to allow forks to make comments on PRs), it’s going to fail until after this is merged – it uses the workflow file from the target branch (trunk), and that means it gets the old GHC (& Cabal) version.

Ok but the current build failures aren't nix related right?
They're something weird about the optimization checks that needs to get fixed for ghc 9.10.3; so not currently mergeable.

@dolio I tried running the benchmarks, but they don’t seem to build

⚙️   Processing stanza 1 of 15.
  ❓

    An error occurred while running the following file: unison-src/transcripts-manual/benchmarks.md

    ``` ucm :hide
    > pull unison.public.base.releases.M4d base
    > pull runarorama.public.sort.data sort
    ```

    🛑

    The transcript failed due to an error in the stanza above. The error is:

    ```
    I think you want to merge unison.public.base.releases.M4d into
    the base branch, but it doesn't exist. If you want, you can
    create it with `branch.empty base`, and then `pull` again.
    ```

At first I thought I was just doing something wrong, but I’ve had the same thing happen on various releases. Granted, it’s still possibly I’m doing something wrong. I‘m now seeing if it works for me at the point the benchmarks were last updated. If not, then it’s very likely I’m doing something wrong.

Either way, the benchmarks should probably be run as part of CI. We don’t need to actually measure them in CI (GitHub is no good for that), but at least ensure they build, so then I’d immediately know what I’m doing wrong.

> pull unison.public.base.releases.M4d base

This is ancient syntax, so those aren't the right benchmarks. Those should be updated or deleted.

@dolio what are the right benchmarks that @sellout should run? Is unison-src/transcripts-manual/benchmarks.md a thing? Or, can you coordinate with them, thanks.

That test checks that GHC is generating code that looks properly optimized. I think it basically checks that the eval function contains no mention of the Stack type.

So, the details of the optimization have changed. I guess you could try running benchmarks before and after to see if the difference actually matters. If it doesn't, then maybe we need a different test. But if it does, we need to figure out how to fix the performance. Even if it doesn't, it might be easier to tweak something to satisfy the check than to figure out a new check that's actually proper.

The benchmarks are on share

https://share.unison-lang.org/@pchiusano/misc-benchmarks

run suite after pulling.

I don't actually remember what is in the benchmark transcript.

Here are the benchmarks (times in µs). With one nasty exception, things look good. I re-ran them to see if there was some interference with that outlier, but everything looked the same.

❌ 3 265.559 10 221.946 213.02% Value.serializeUncompressed (10k element map)

For what it's worth, this benchmark looks to be using Value.serialize_v4. Most things would be using v5 (Value.serialize) these days, so that would probably be the more relevant benchmark. It probably isn't a big deal if v4 serialization gets twice as slow. Though it's also strange because the compressed version of the test still using serialize_v4 and then just sends it through zlib compression/decompression. That makes it seem like either a fluke or that there was a boost in the zlib performance that is offsetting a performance regression in the v4 serialization.