fix data to use managed memory instead of naked pointer

[cometkim]

as following https://github.com/ocaml/ocaml/issues/13141

I tried to fix it but let me ask if this doesn't ruin your intentions.

[dmtrKovalenko]

Thanks for your investigation I been to dive into this.

Did you try to run benchmarks for this? And also measure GC stats in the end comparing to the main branch?

[cometkim]

Did you try to run benchmarks for this? And also measure GC stats in the end comparing to the main branch?

I'm not sure if I did it right, but it doesn't seem to make a noticeable difference on a simple usecase.

❯ hyperfine -i './odiff-main.exe images/donkey.png images/donkey-2.png 2>/dev/null' './odiff-fix.exe images/donkey.png images/donkey-2.png 2>/dev/null'
Benchmark 1: ./odiff-main.exe images/donkey.png images/donkey-2.png 2>/dev/null
  Time (mean ± σ):     203.9 ms ±   1.7 ms    [User: 125.9 ms, System: 77.3 ms]
  Range (min … max):   202.0 ms … 207.6 ms    14 runs

  Warning: Ignoring non-zero exit code.

Benchmark 2: ./odiff-fix.exe images/donkey.png images/donkey-2.png 2>/dev/null
  Time (mean ± σ):     204.1 ms ±   0.6 ms    [User: 132.7 ms, System: 71.3 ms]
  Range (min … max):   203.0 ms … 205.1 ms    14 runs

  Warning: Ignoring non-zero exit code.

Summary
  ./odiff-main.exe images/donkey.png images/donkey-2.png 2>/dev/null ran
    1.00 ± 0.01 times faster than ./odiff-fix.exe images/donkey.png images/donkey-2.png 2>/dev/null

Are there any scenarios where GC can be critical?

[cometkim]

If the intention is to use the codec's buffer as is, I think a completely different approach is also possible.

Instead of copying the buffer into a bigarray, create a custom block pointer and register the finalizer on it. Although odiff will be less dependent on OCaml, it will boot faster with less copying.

Either way, it will have roughly the same lifetime as the program, so major GC (of the image data) will occur rarely I guess.

---

Note: This change should not trigger additional GC, it just moves the buffer's location.

[gasche]

I didn't review the details but the change broadly matches what I had in mind. Did you manage to test that the project now works with OCaml 5 ?

[cometkim]

Yep, i tested it for work with OCaml 5 in my branch https://github.com/cometkim/odiff/tree/ocaml5

[cometkim]

btw the OCaml 5 build is about 10% larger (5_148_720 -> 5_725_304 bytes)

perf is not changed

❯ hyperfine --warmup 3 -i './ODiffBin-main images/water-4k.png images/water-4k-2.png 2>/dev/null'  './ODiffBin-ocaml5 images/water-4k.png images/water-4k-2.png 2>/dev/null'

Benchmark 1: ./ODiffBin-main images/water-4k.png images/water-4k-2.png 2>/dev/null
  Time (mean ± σ):      1.003 s ±  0.011 s    [User: 0.779 s, System: 0.223 s]
  Range (min … max):    0.993 s …  1.024 s    10 runs

  Warning: Ignoring non-zero exit code.

Benchmark 2: ./ODiffBin-ocaml5 images/water-4k.png images/water-4k-2.png 2>/dev/null
  Time (mean ± σ):      1.016 s ±  0.007 s    [User: 0.792 s, System: 0.222 s]
  Range (min … max):    1.010 s …  1.032 s    10 runs

  Warning: Ignoring non-zero exit code.

Summary
  ./ODiffBin-main images/water-4k.png images/water-4k-2.png 2>/dev/null ran
    1.01 ± 0.01 times faster than ./ODiffBin-ocaml5 images/water-4k.png images/water-4k-2.png 2>/dev/null

[cometkim]

Great! Migrating to OCaml 5 is now very simple.

I haven't tried parallelization yet, but I guess it shouldn't be too difficult. Considering the anti-aliasing feature, some pixels may be processed redundantly, or additional optimization may be required.

[dmtrKovalenko]

Tried to update the ocaml 5 but I am getting esy permissoin issues. I see you have ocaml5 branch in your fork, were you able to build it with esy @cometkim ?

[cometkim]

yep, I already did upstream it https://github.com/reasonml/reason-native/pull/271

let me file a PR to this repo too

[cometkim]

error: build failed with exit code: 1
  build log:
    # esy-build-package: building: @opam/ocamlfind@opam:1.9.6
    # esy-build-package: pwd: /Users/tim/.esy/3/b/opam__s__ocamlfind-opam__c__1.9.6-975bcba1
    # esy-build-package: running: 'patch' '--strip' '1' '--input' '0001-Harden-test-for-OCaml-5.patch'
    patch: **** patch file 0001-Harden-test-for-OCaml-5.patch not found: No such file or directory
    error: command failed: 'patch' '--strip' '1' '--input' '0001-Harden-test-for-OCaml-5.patch' (exited with 2)
    esy-build-package: exiting with errors above...

  building @opam/ocamlfind@opam:1.9.6
esy: exiting due to errors above

resolving esy dependencies with bunch of overrides is non-trivial .. 😅 I'm fixing effects that started appearing in a completely unrelated place after changing the esy.lock.

[cometkim]

Found this https://github.com/ocaml/opam-repository/issues/25961

I think I should try again after rm -rf ~/.opam/repo/default && opam update default 😅