Run things under both regular lua *and* yklua.

[vext01]

We use the same exact source code for both, just one is built with JIT support, the other not.

[ltratt]

Why is this a problem? We're not benchmarking the time it takes to build yklua are we? By the time the binary is built, and assertions in ykllvm are irrelevant I think, unless assertions are embedded in yklua?

[vext01]

Ah yes. It's burnt into my head that we are using the LLVM API at runtime, but we are not!

OK, so should my next PR be to use the cached ykllvm? Part of me still wants to build it from scratch, but I don't feel strongly.

[ltratt]

It's fine not to use a cached ykllvm here if you feel strongly about it, but let's nuke the comment either way.

[vext01]

I've killed the comment in c030728

[ltratt]

Should this be lua/src or ...?

[vext01]

yklua/src is correct.

[ltratt]

Oh, hang on: this means we're not benchmarking lua but yklua in two different configs. Note that "yklua without yk" will not, over time, be quite the same thing as "yk". Right now, if you don't #define USE_YK you should end up (more-or-less) with the same binary, but I think that's not going to end up being viable when we start doing things like rewriting functions to expose elidable functions etc.

That suggests to me that we should build a "normal" Lua, from source, that's not derived directly from yklua.

[vext01]

I thought it would be unfair to not use the same source repo, but now I'm not so sure.

I was working under the assumption that all code we added to the lua source code is guarded with USE_YK.

If that's not true we should indeed download a separate "no yk changes in sight" source tree and use that as a baseline.

Maybe we should just do that anyway for peace of mind. OK to do that in another PR?

[ltratt]

I was working under the assumption that all code we added to the lua source code is guarded with USE_YK.

It is right now (probably...) but I doubt that will be the case going forwards. [I'm actually unsure whether USE_YK is massively useful even now... it does help a bit with reading the code, but you can always diff against "true Lua" to see the diffs, which is probably a better thing to do anway.]

If that's not true we should indeed download a separate "no yk changes in sight" source tree and use that as a baseline.

Maybe we should just do that anyway for peace of mind. OK to do that in another PR?

Yes and yes respectively.

[ltratt]

Please squash.

[vext01]

squashed.

[vext01]

Looks like I messed up the rust install. Investigating.

[vext01]

OK to squash?

[ltratt]

Please squash.

[vext01]

Ready.

[vext01]

Ah hah!

 munmap_chunk(): invalid pointer

So I'm not losing my mind after all! ykjit/yk#1326

Do you want to retry?

[ltratt]

The munmap might be a red herring. Look at this second error:

18:18:02 thread '<unnamed>' panicked at ykrt/src/compile/jitc_yk/trace_builder.rs:544:25:
18:18:02 explicit panic
18:18:02 note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
18:18:02 munmap_chunk(): invalid pointer
18:18:02 Aborted (core dumped)

which is this code:

                    jit_ir::Operand::Const(_) => {
                        // Since we are forcing constants into `ProxyConst`s during inlining, this
                        // case should never happen.
                        panic!()
                    }

We should try this with RUST_BACKTRACE=1. [All that said, a likely problem is stackmaps / ykllvm in some way. We'll need to keep an open mind.]

[vext01]

This time it failed due to:

19:05:33         Starting Havlak benchmark ...
19:05:33         thread '<unnamed>' panicked at ykrt/src/compile/jitc_yk/trace_builder.rs:544:25:
19:05:33         explicit panic
19:05:33         note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
19:05:33         thread '<unnamed>' panicked at ykrt/src/compile/jitc_yk/trace_builder.rs:544:25:
19:05:33         explicit panic
19:05:33         Segmentation fault (core dumped)

i.e. not memory corruption.

We've seen this one before, but I was unable to find an issue. Will raise one.

[vext01]

Issue here:
ykjit/yk#1340

Do we want to try this once more while I look into a lead @ltratt gave me this morning on why there could be memory corruption?

[vext01]

Since properly disabling side-tracing I've had clean runs of these benchmarks (when running locally with try_repeat).

I think there's a good chance this will merge if we retry.

[vext01]

0:10:58 Executing run:Havlak (YkLua, awfy, 1500) default None None None
10:10:58     cmd: /ci/yklua/src/yklua  harness.lua Havlak 1  1500
10:10:58     cwd: /ci/awfy/Lua
10:10:58     Run failed. Return code: 139
10:10:58     Output:
10:10:58 
10:10:58         Starting Havlak benchmark ...
10:10:58         thread '<unnamed>' panicked at ykrt/src/compile/jitc_yk/trace_builder.rs:544:25:
10:10:58         explicit panic
10:10:58         note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
10:10:58         Segmentation fault (core dumped)
10:10:58         
10:10:58     Execution has failed, benchmark is aborted.
10:10:58         The benchmark failed 1 times in a row.

Back to trying to reproduce this locally then...

[ltratt]

@vext01 My guess is that this error is a "genuine" one that you can replicated with synchronised compilation, and probably reflects a genuine bug/limitation in trace_builder.

[vext01]

I've managed to repro this locally. Will investigate.

[vext01]

@ltratt with ykjit/yk#1369 merged, this should now succeed.

(I've tested a previously failing benchmark now works locally)

[vext01]

Uff

14:35:56         Starting Havlak benchmark ...
14:35:56         Segmentation fault (core dumped)

This PR will be my demise.

[vext01]

With the memory corruption bug fixed, and with any luck, this may merge now. 🙏