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

Julia 1.7 performance regression on JPEG image #208

Closed
johnnychen94 opened this issue Dec 1, 2021 · 8 comments
Closed

Julia 1.7 performance regression on JPEG image #208

johnnychen94 opened this issue Dec 1, 2021 · 8 comments

Comments

@johnnychen94
Copy link
Member

CRef: JuliaImages/Images.jl#992

The following patch fixes the performance issue, but I'm lost in the memory orders...

diff --git a/src/ImageMagick.jl b/src/ImageMagick.jl
index db26f64..b0c36fb 100755
--- a/src/ImageMagick.jl
+++ b/src/ImageMagick.jl
@@ -149,13 +149,14 @@ function load_(file::Union{AbstractString,IO,Vector{UInt8}}, permute_horizontal=
     sz, T, cs, channelorder = _metadata(wand)
 
     # Allocate the buffer and get the pixel data
-    buf = Array{T}(undef, sz)
-    exportimagepixels!(rawview(channelview(buf)), wand, cs, channelorder)
+    buf = Array{UInt8}(undef, (length(T), sz...))
+    exportimagepixels!(buf, wand, cs, channelorder)
 
     orient = getimageproperty(wand, "exif:Orientation", false)
     default = (A,ph) -> ph ? vertical_major(A) : identity(A)
     oriented_buf = get(orientation_dict, orient, default)(buf, permute_horizontal)
-    view ? oriented_buf : collect(oriented_buf)
+    colorful_buf = length(T) == 1 ? reinterpret(T, reshape(buf, sz)) : reinterpretc(T, buf)
+    view ? colorful_buf : collect(colorful_buf)
 end

cc: @etibarg

@johnnychen94
Copy link
Member Author

See also JuliaImages/ImageCore.jl#174

@yakir12
Copy link

yakir12 commented Dec 16, 2021

I can confirm. It takes 1.5 seconds to load a 6 MB image

using FileIO, Colors
img = rand(RGB, 1440, 2560)
file = "img.jpg"
save(file, img)

using BenchmarkTools
filesize(file) # 6422686 bytes
@btime load(file); #  1.478 s (44236892 allocations: 1.51 GiB)

on

(tmp) pkg> status
      Status `~/tmp/Project.toml`
  [5789e2e9] FileIO v1.11.2
  [6218d12a] ImageMagick v1.2.2

julia> versioninfo()
Julia Version 1.7.0
Commit 3bf9d17731 (2021-11-30 12:12 UTC)
Platform Info:
  OS: Linux (x86_64-linux-gnu)
  CPU: AMD Ryzen Threadripper 2950X 16-Core Processor
  WORD_SIZE: 64
  LIBM: libopenlibm
  LLVM: libLLVM-12.0.1 (ORCJIT, znver1)

@johnnychen94
Copy link
Member Author

Upstream patch JuliaLang/julia#43347 should fixes this, so keep patient on Julia 1.7.1...

@lorenzoh
Copy link

Just checked on 1.7.1, the problem persists

@yakir12
Copy link

yakir12 commented Dec 29, 2021

Yeah, can confirm, it is not resolved in 1.7.1.

For my MWE above I still get:

julia> @btime load(file);
  1.680 s (44236892 allocations: 1.51 GiB)

@johnnychen94
Copy link
Member Author

Unfortunately that PR isn't merged yet. I'll see if I can continue https://github.com/stevengj/JpegTurbo.jl and bring up the JPEG wrapper before Feb.

@johnnychen94
Copy link
Member Author

I'll see if I can continue https://github.com/stevengj/JpegTurbo.jl and bring up the JPEG wrapper before Feb.

Almost ready now. Just need to wait for a few days before registering it. Feel free to try https://github.com/johnnychen94/JpegTurbo.jl out.

@johnnychen94
Copy link
Member Author

A better solution is now available at https://github.com/johnnychen94/JpegTurbo.jl. For ImageMagick itself, this should be fixed for Julia 1.8.

I currently have very little interest in maintaining the old v6 version. Our plan is to gradually deprecate this package with the newer ImageIO.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants