Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: performance degradation after naked pointer fix #106

Merged
merged 1 commit into from
Aug 26, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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))))


Loading