Skip to content

Commit

Permalink
fix: performance degradation after naked pointer fix (#106)
Browse files Browse the repository at this point in the history
This reverts commit 05447f4.
  • Loading branch information
dmtrKovalenko authored Aug 26, 2024
1 parent 6fa540f commit 2037b8a
Show file tree
Hide file tree
Showing 13 changed files with 43 additions and 19 deletions.
16 changes: 12 additions & 4 deletions .github/workflows/build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ jobs:
env:
VCPKG_DEFAULT_TRIPLET: ${{ matrix.triplet }}
VCPKG_DEFAULT_HOST_TRIPLET: ${{ matrix.triplet }}
VCPKG_BUILD_TYPE: release
with:
runVcpkgInstall: true
runVcpkgFormatString: '["install", "--clean-after-build"]'
Expand Down Expand Up @@ -83,9 +84,6 @@ jobs:
- run: opam exec -- opam install . --deps-only --with-test
- run: opam exec -- dune build --verbose

- name: Give binary system-specific name
run: cp "_build/default/bin/ODiffBin.exe" "odiff-${{ matrix.artifact }}.exe"

- run: opam exec -- dune exec ODiffBin -- --version
- run: opam exec -- dune runtest

Expand All @@ -106,11 +104,21 @@ jobs:
run: npm ci
- name: e2e test
run: npm test

- name: Build release binary
# this is needed because now ocaml's internal cygwin will be used on windows
# which breaks normal line endings
env:
SHELLOPTS: igncr
run: |
opam exec -- dune clean && \
opam exec -- dune build --release && \
cp "_build/default/bin/ODiffBin.exe" "odiff-${{ matrix.artifact }}.exe"
- if: always()
uses: actions/upload-artifact@v4
with:
if-no-files-found: ignore
if-no-files-found: error
name: odiff-${{ matrix.artifact }}.exe
path: odiff-${{ matrix.artifact }}.exe
retention-days: 14
Expand Down
19 changes: 11 additions & 8 deletions bin/Main.ml
Original file line number Diff line number Diff line change
Expand Up @@ -20,19 +20,22 @@ let main img1Path img2Path diffPath threshold outputDiffMask failOnLayoutChange
diffColorHex toEmitStdoutParsableString antialiasing ignoreRegions diffLines
disableGcOptimizations =
(*
We do not need to actually maintain memory size of the allocated RAM by odiff, so we are
increasing the minor memory size to avoid most of the possible deallocations. For sure it is
not possible be sure that it won't be run in OCaml because we allocate the Stack and Queue
Increase amount of allowed overhead to reduce amount of GC work and cycles.
we target 1-2 minor collections per run which is the best tradeoff between
amount of memory allocated and time spend on GC.
By default set the minor heap size to 256mb on 64bit machine
For sure it depends on the image size and architecture. Primary target x86_64
*)
if not disableGcOptimizations then
Gc.set
{
(Gc.get ()) with
Gc.minor_heap_size = 64_000_000;
Gc.stack_limit = 2_048_000;
Gc.window_size = 25;
(* 128 MB *)
major_heap_increment = 128 * 1024 * 1024;
(* Reasonable high value to reduce major GC frequency *)
space_overhead = 500;
(* Disable compaction *)
max_overhead = 1_000_000;
};

let module IO1 = (val getIOModule img1Path) in
Expand Down Expand Up @@ -64,5 +67,5 @@ let main img1Path img2Path diffPath threshold outputDiffMask failOnLayoutChange
| Some output when outputDiffMask -> IO1.freeImage output
| _ -> ());

(*Gc.print_stat stdout;*)
(* Gc.print_stat stdout; *)
exit exitCode
2 changes: 1 addition & 1 deletion dune-project
Original file line number Diff line number Diff line change
Expand Up @@ -12,14 +12,14 @@

(generate_opam_files true)

(version 0.3.1-rc.3)
(source (github dmtrKovalenko/odiff))
(license MIT)
(authors "Dmitriy Kovalenko")
(maintainers "https://dmtrkovalenko.dev" "[email protected]")

(package
(name odiff)
(version 0.3.1-rc.3)
(synopsis "CLI for comparing images pixel-by-pixel")
(depends
odiff-core
Expand Down
Binary file modified images/water-diff.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
2 changes: 1 addition & 1 deletion io/jpg/dune
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
(language c)
(names ReadJpg)
(flags
(:include jpg_c_flags.sexp)))
(:include jpg_c_flags.sexp) -O3))
(c_library_flags
(:include jpg_c_library_flags.sexp))
(libraries odiff-core WritePng))
Expand Down
5 changes: 3 additions & 2 deletions io/png/ReadPng.c
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ CAMLprim value read_png_file(value file) {

ctx = spng_ctx_new(0);
if (ctx == NULL) {
fclose(png);
fclose(png);
caml_failwith("spng_ctx_new() failed");
}

Expand Down Expand Up @@ -56,7 +56,8 @@ CAMLprim value read_png_file(value file) {
caml_failwith(spng_strerror(result));
};

ba = caml_ba_alloc(CAML_BA_UINT8 | CAML_BA_C_LAYOUT | CAML_BA_MANAGED, 1, NULL, &out_size);
ba = caml_ba_alloc(CAML_BA_UINT8 | CAML_BA_C_LAYOUT | CAML_BA_MANAGED, 1,
NULL, &out_size);
unsigned char *out = (unsigned char *)Caml_ba_data_val(ba);

result =
Expand Down
2 changes: 1 addition & 1 deletion io/png/dune
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
(language c)
(names ReadPng)
(flags
(:include png_c_flags.sexp)))
(:include png_c_flags.sexp) -O3))
(c_library_flags
(:include png_c_library_flags.sexp))
(libraries odiff-core WritePng))
Expand Down
2 changes: 1 addition & 1 deletion io/png_write/dune
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
(language c)
(names WritePng)
(flags
(:include png_write_c_flags.sexp)))
(:include png_write_c_flags.sexp) -O3))
(c_library_flags
(:include png_write_c_library_flags.sexp)))

Expand Down
2 changes: 1 addition & 1 deletion io/tiff/dune
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
(language c)
(names ReadTiff)
(flags
(:include tiff_c_flags.sexp)))
(:include tiff_c_flags.sexp) -O3))
(c_library_flags
(:include tiff_c_library_flags.sexp))
(libraries odiff-core WritePng))
Expand Down
1 change: 1 addition & 0 deletions odiff-core.opam
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
# This file is generated by dune, edit dune-project instead
opam-version: "2.0"
version: "0.3.1-rc.3"
synopsis: "Pixel-by-pixel image difference algorithm"
maintainer: ["https://dmtrkovalenko.dev" "[email protected]"]
authors: ["Dmitriy Kovalenko"]
Expand Down
1 change: 1 addition & 0 deletions odiff-io.opam
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
# This file is generated by dune, edit dune-project instead
opam-version: "2.0"
version: "0.3.1-rc.3"
synopsis: "Ready to use io for odiff-core"
maintainer: ["https://dmtrkovalenko.dev" "[email protected]"]
authors: ["Dmitriy Kovalenko"]
Expand Down
1 change: 1 addition & 0 deletions odiff-tests.opam
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
# This file is generated by dune, edit dune-project instead
opam-version: "2.0"
version: "0.3.1-rc.3"
synopsis: "Internal package for integration tests of odiff"
maintainer: ["https://dmtrkovalenko.dev" "[email protected]"]
authors: ["Dmitriy Kovalenko"]
Expand Down
9 changes: 9 additions & 0 deletions src/dune
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,12 @@
(public_name odiff-core)
(flags
(-w -40 -w +26)))

(env
(dev
(flags (:standard -w +42))
(ocamlopt_flags (:standard -S)))
(release
(ocamlopt_flags (:standard -O3 -rounds 5 -unbox-closures -inline 200 -inline-max-depth 7 -unbox-closures-factor 50))))


0 comments on commit 2037b8a

Please sign in to comment.