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

Image loading is slow #7

Open
SuperDisk opened this issue Dec 28, 2024 · 5 comments
Open

Image loading is slow #7

SuperDisk opened this issue Dec 28, 2024 · 5 comments

Comments

@SuperDisk
Copy link

Loading a 1920x1080 image with

(magick:with-magick-wand (wand)
  (time
   (magick:read-image-blob wand (alexandria:read-file-into-byte-vector "/tmp/my-img.bmp"))))

takes around 500 ms on my machine. I wrote a simple C program to do the same thing, and it completes in around 1 ms. Any idea why it would be so much slower? I did a quick glance into the code but it seems like it's a pretty thin wrapper over MagickReadImageBlob.

@SuperDisk
Copy link
Author

Ah, I found it's because the library injects code to do a very slow byte-by-byte copy into a foreign allocated vector before passing it to ImageMagick.

(cffi:defcfun "MagickReadImageBlob" :boolean
  (wand :pointer)
  (blob :pointer)
  (length :int))


(magick:with-magick-wand (wand)
  (let ((vec (alexandria:read-file-into-byte-vector "/tmp/bigdump.bmp")))
    (cffi:with-pointer-to-vector-data (ptr vec)
      (time
       (magickreadimageblob wand ptr (length vec)))
      (magick:write-image wand "/tmp/mydump.bmp "))))

This code in contrast takes around 3ms.

@SuperDisk
Copy link
Author

I think the function in the library, as-is also leaks memory or something, since even just a few loads can cause a blowup in heap usage. The small cffi wrapper I posted above doesn't have this problem.

@ruricolist
Copy link
Owner

Do you want to make PR with this change?

@SuperDisk
Copy link
Author

SuperDisk commented Dec 28, 2024

I'd be happy to, but I'm actually not sure what the best way to do it is. I just slapped the CFFI definition in my own code and started using it, but the defmagickfun stuff is complicated and I'm not sure if it would play nice to just insert this definition. Basically any function that's declared to take a dynarray could use this optimization, it's just that the image loading seemed to be the part that was causing me really annoying slowdown.

Also, technically cffi:with-pointer-to-vector-data is specified to only take vectors made with cffi:make-shareable-byte-vector, but it happens that on SBCL cffi:make-shareable-byte-vector just calls make-array with no special behavior, so this works. On ABCL for instance it would be problematic I think.

@ruricolist
Copy link
Owner

I'm wary of making major changes to a library (1) without a test suite and (2) that I no longer personally use, but in principleb it seems like it would be a useful optimization to make defmagickfun expand to pass the array without a copy on SBCL.

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

No branches or pull requests

2 participants