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

NewFromBuffer causes GLib-ERROR with x86 libvips #26

Closed
jjonesrs opened this issue Jan 19, 2019 · 11 comments
Closed

NewFromBuffer causes GLib-ERROR with x86 libvips #26

jjonesrs opened this issue Jan 19, 2019 · 11 comments
Labels
bug Something isn't working
Milestone

Comments

@jjonesrs
Copy link

Hi @kleisauke, thanks a ton for this libvips wrapper. I've learned a lot from reading your correspondence with @jcupitt.

Based on the advice in https://images.guide/, I'm currently reimplementing an image optimization pipeline with libvips at the core. Unfortunately, the larger program that this is a part of references other native x86 assemblies so for now I'm working with x86 libvips.

The x86 3-4GB memory limit seems to be causing an error that looks like (NetVips:17212): GLib-ERROR **: 11:19:21.666: /data/mxe/tmp-glib-i686-w64-mingw32.shared/glib-2.59.0/glib/gmem.c:105: failed to allocate 39493103 bytes.

image

I reproduced this issue in an isolated project and included large and small test images. https://github.com/jjonesrs/NetVipsLeakTest

Here it is inline:

byte[] imageBytes = File.ReadAllBytes(@"large-test-image.png");
//byte[] imageBytes = File.ReadAllBytes(@"small-test-image.jpg");

Base.LeakSet(1);

Operation.VipsCacheSetMax(0);
Operation.VipsCacheSetMaxFiles(0);

for (int i = 0; i < 1000000; i++)
{
    using (Image img = Image.NewFromBuffer(imageBytes))
    {
        Console.WriteLine(img);
    }
}

The above small snippet might look unrealistic, but it intentionally doesn't do any more than call Image.NewFromBuffer() on the source image to serve as a test of loading from a buffer in rapid succession. Loading from a file understandably does not exhibit the same memory patterns, but I'd highly prefer not to pull images from S3, save them to local disk, then risk file lock contention on delete.

I would expect the using ... statement to call Dispose() and signal to libvips to free the native memory at some point before the GLib crash, but it doesn't seem to have any effect.

I don't think that NetVips or libvips is strictly leaking, but it looks like maybe libvips isn't freeing images fast enough to keep up with the low x86 memory limit. Testing under x64, I can't reproduce the GLib error, but I do observe (in my opinion) high memory usage of 2-4GB.

I've included two memory profiles from JetBrains dotMemory leading up to the crash.

large-test-image.png:
image

small-test-image.jpg:
image

@jcupitt
Copy link
Contributor

jcupitt commented Jan 19, 2019

I had a quick go in Python:

import sys
import pyvips

pyvips.cache_set_max(0)

buf = open(sys.argv[1], "rb").read()

for i in range(0, 100000):
    if i % 100 == 0:
        print("loop {} ...".format(i))
        
    image = pyvips.Image.new_from_buffer(buf, "")

And I see a steady 45mb of RES used.

If you are seeing 2 - 4gb, I'd guess the GC isn't running often enough.

kleisauke added a commit that referenced this issue Jan 20, 2019
Use `AddMemoryPressure` and `RemoveMemoryPressure` to inform the GC about the amount of native memory actually used by the object. Since the garbage collector cannot track the memory allocated by unmanaged code, it only tracks the memory allocated by managed code.

Also made a number of refactors (switched to `System.Span<T>` where possible, avoid unnecessary `Marshal.Copy` calls and removed some unused logging wrappers).
@kleisauke kleisauke added the bug Something isn't working label Jan 20, 2019
@kleisauke
Copy link
Owner

Hi @jjonesrs,

I could reproduce this and fix it with commit 93f1002. It seems that a GC.AddMemoryPressure call was missing in GValue when allocating a VipsBlob type. GC.AddMemoryPressure ensures that GC knows the true cost of the object during collection. If the object is actually bigger than the managed size reflects, it may be a candidate for quick(er) collection.

If you want to test this, you can use the nightly version of NetVips. Add the https://ci.appveyor.com/nuget/net-vips feed in the <packageSources> section of your NuGet.config:

<packageSources>
  <add key="netvips-nightly" value="https://ci.appveyor.com/nuget/net-vips" />
</packageSources>

And update NetVips to 1.0.8. I will try to make a new release next month (hopefully also with #21 included).

Thank for reporting this and providing an isolated project!

@kleisauke kleisauke added this to the 1.1.0 milestone Mar 9, 2019
@kleisauke
Copy link
Owner

I've just released NetVips 1.1.0-rc1 and NetVips.Native 8.8.0-rc1 (which contains the pre-compiled libvips 8.8.0-rc1 binaries for Linux, macOS and Windows) on NuGet. This release should fix the GC issue described above.

If you would like to see what's new in libvips 8.8, please visit the the release notes of libvips:
https://libvips.github.io/libvips/2019/04/22/What's-new-in-8.8.html

I'm sorry it took so long to release this. By separating the pre-compiled libraries from the binding itself, it's now easier to release a new version.

@jjonesrs
Copy link
Author

Hey @kleisauke, thank you for the fixes. I have another inquiry unrelated to the above, but I'll post it here until I know of a better place.

I previously compiled libvips using a hacked up version of https://github.com/libvips/build-win64-mxe to include support for mozjpeg. Could you please add official support for mozjpeg in one of the Windows build repos? After reading through the latest issues and PRs, I'm not sure which build method will be used going forward. Also, mozjpeg's in a bit of a transition with their build tools as well, so basically I'm a bit lost.

@kleisauke
Copy link
Owner

I don't intend to switch the pre-compiled binaries to MozJPEG, since image compression in MozJPEG is slower than with libjpeg-turbo, see:
lovell/sharp#144 (comment)

The pre-compiled binaries in NuGet are intended to get people up and running as quickly as possible with the most popular web-based formats. You can always roll your own combination of libvips' dependencies, NetVips will always look for a global installation if the NetVips.Native NuGet package isn't installed.

I could add support for MozJPEG in the build-win64-mxe repo (I created libvips/build-win64-mxe#8 for this). NetVips will still use the build-win64-mxe repo for the pre-compiled Windows binaries as it supports both 32-bit and 64-bit versions of Windows and static linkage (which reduces the amount of DLLs).

@kleisauke
Copy link
Owner

@jjonesrs I just added support for MozJPEG in the Windows build repo of libvips (commit libvips/build-win64-mxe@62bdc7c). The binaries can be download here.

@kleisauke
Copy link
Owner

NetVips v1.1.0 is now available. Thanks for reporting this @jjonesrs.

@kDanil
Copy link

kDanil commented Jan 9, 2020

Hi @kleisauke, tnx for your work.

I'm having troubles with using large images on Linux. It seems issue described above is not fully fixed on Linux.
On Windows, though, works fine without memory degradation.

Setup:
.NET Core 3.0 C# project
NetVips version 1.1.0
NetVips.Native version 8.8.4
Running with docker and using mcr.microsoft.com/dotnet/core/aspnet:3.0 image

NetVips.NetVips.CacheSetMaxMem(0);
NetVips.NetVips.CacheSetMax(0);
NetVips.NetVips.CacheSetMaxFiles(0);
NetVips.NetVips.LeakSet(true);

Per web request I'm reading large JPG from file and perform 30 image initializations and force GC to collect all resources in the end.

Code:

var buffer = System.IO.File.ReadAllBytes(@"1041615.jpg");
for (var i = 0; i < 30; i++)
{
    using (var image = Image.NewFromBuffer(buffer))
    {
        Console.WriteLine($"memory processing {image}");
    }

    await Task.Delay(1000);
}
GCSettings.LargeObjectHeapCompactionMode = 
GCLargeObjectHeapCompactionMode.CompactOnce;
GC.Collect();
GC.WaitForPendingFinalizers();

For 7mb image and one web request (30 image initialisations) working set is growing above 200mb and with final GC call shrink to 178 Mb.
netvips memory leak linux 7mb image
netvips memory leak linux 7mb image leak profiler

For 14mb image and 5 web requests (150 image initialisations) working set is growing above 1gb. GC call at the end of each request helping a bit, but not much.
netvips memory leak linux 14mb image
netvips memory leak linux 14mb image leak profiler

Any ideas?

@kDanil
Copy link

kDanil commented Jan 9, 2020

All above was on Debian (mcr.microsoft.com/dotnet/core/aspnet:3.0)
When I tried Alpine (mcr.microsoft.com/dotnet/core/aspnet:3.0.1-alpine3.10)
memory is dropping to initial 100Mb after GC call, which is what expected

So it might be related to some lib implementation per distribution.

@kleisauke
Copy link
Owner

Hello @kDanil,

I wonder if this is the effect of memory fragmentation. Given this is Linux, are you able to try with a different memory allocator such as jemalloc?

You might be interested in lovell/sharp#955 to learn more about memory allocation and why the choice of a memory allocator is important.

@kDanil
Copy link

kDanil commented Jan 27, 2020

Hello, @kleisauke, thank you for a link.

I see that ppl either go to Alpine or changing memory allocator.
Proposed solution with jemalloc is:

RUN apt-get update && apt-get install --force-yes -yy \
  libjemalloc1 \
  && rm -rf /var/lib/apt/lists/*

# Change memory allocator to avoid leaks
ENV LD_PRELOAD=/usr/lib/x86_64-linux-gnu/libjemalloc.so.1

Haven't tried it by myself yet, but I'm curious about advantages/disadvantages of both approaches.
Do you know why way to prefer? Any limitations?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants