Patch to keep larger pixel cache allocations in "Anonymous Memory" maps

[dvdcrn]

We have a long-running daemon process that uses jmagick-6.4.0, which is a Java Native Interface (JNI) into the ImageMagick API. Therefore, we are using the ImageMagick-6.9.4-8 shared library libMagickCore-6.Q16.so.

We encountered a memory leak in the "C" malloc heap because:
(A) We had a code path that wasn't calling the Java-level MagickImage.destroyImages() method that would free the large PixelCache allocations in the "C" malloc heap.
(B) Our java -Xmx setting was sufficiently large, and our java heap Old Gen growth was sufficiently slow, that we never encountered a major GC run.

Although our MagickImage java objects weren't live, they wouldn't be finalized until the major GC run that never came, so the outstanding PixelCache allocations weren't being free back into the heap.

Surprisingly, even though have fixed (A), we still see the daemon process RSS grow constantly. This only stops if we fix (B) by reducing the heap size until every GC run is a major GC run. What I now suspect is that the large PixelCache allocations in the "C" malloc heap are being fragmented shortly after being freed, by smaller allocations that aren't freed until the major GC finalizer.

One way to prevent these sorts of problems is to use an "Anonymous Memory" allocation (using the mmap function in the "C" system library using the MAP_ANONYMOUS and MAP_PRIVATE flags) instead of a standard "Heap Memory" allocation (using the malloc function). This is already built into ImageMagick, but seemingly not as an alternative for PixelCache.

I developed a patch whereby I can force large PixelCache blobs to allocate as an "Anonymous Memory" blob with a new pixel resource limit called "heap" that can be configured in the usual ways (policy.xml, env var, command-line flag). It is similar to the "area" limit, being a configurable byte threshold that will change the algorithm for allocation of a single PixelCache.

In the "resource.html" documentation, the "area" limit is described: "Any image larger than this area limit is cached to disk rather than memory."

The new "heap" limit would be described: "Any image larger than this byte limit (but smaller than the area limit) is cached in an anonymous memory map rather than heap memory."

This feature would be very helpful in preventing malloc heap fragmentation in long-running processes linked against libMagickCore-6.Q16.so. An anonymous memory map can be completely freed back into the operating system, since it's not embedded in a larger memory address range.

For example, here are the pixel resource limits that we are now running with in our policy.xml file:

Code: Select all

  <!--
    The per-image (original or intermediate) limits:
     * All pixel cache images will be Anonymous Memory (heap=50KiB)
     * Any 16+ MP image will be in Disk Cache (area=123MiB)
    The overall processing (for all images) limits (for our resizing code):
     * One 16 MP photo could process in Memory (memory=256MiB)
     * Another 16 MP photo could process as mapped files (map=256MiB)
     * One 26 MP photo is the hard limit (one 199MiB in memory, another on disk=400MiB)
    -->

  <policy domain="resource" name="heap" value="50KiB"/>
  <policy domain="resource" name="area" value="123MiB"/>
  <policy domain="resource" name="memory" value="256MiB"/>
  <policy domain="resource" name="map" value="256MiB"/>
  <policy domain="resource" name="disk" value="400MiB"/>

Thanks for considering this feature. I'll cut-and-paste my patch against the ImageMagick-6.9.4-8 code base. I even changed the documentation files.

Thanks,
David Crane
Director of Engineering, DonorsChoose.org

--------------------
To apply my patch (which is below)

Code: Select all

tar xsvf ImageMagick-6.9.4-8.tar.gz
cd ImageMagick-6.9.4-8
patch -s -p0 < ../imagemagick-anonymous-memory-maps.patch

--------------------
And here is my patch, which you'd save in the ../imagemagick-anonymous-memory-maps.patch file

Code: Select all

diff -ruN ImageMagick-6.9.4-8/magick/resource_.h              magick/resource_.h
--- ImageMagick-6.9.4-8/magick/resource_.h                    2016-06-07 09:28:31.000000000 -0400
+++ magick/resource_.h                                        2016-06-10 18:58:39.000000000 -0400
@@ -26,6 +26,7 @@
 {
   UndefinedResource,
   AreaResource,
+  HeapResource,
   DiskResource,
   FileResource,
   MapResource,
diff -ruN ImageMagick-6.9.4-8/magick/cache.c                  magick/cache.c
--- ImageMagick-6.9.4-8/magick/cache.c                        2016-06-07 09:28:31.000000000 -0400
+++ magick/cache.c                                            2016-06-10 18:58:39.000000000 -0400
@@ -3870,7 +3870,8 @@
     number_pixels;
 
   MagickStatusType
-    status;
+    status,
+    heapNotMap;
 
   size_t
     columns,
@@ -3927,8 +3928,16 @@
         {
           status=MagickTrue;
           cache_info->mapped=MagickFalse;
-          cache_info->pixels=(PixelPacket *) MagickAssumeAligned(
-            AcquireAlignedMemory(1,(size_t) cache_info->length));
+          heapNotMap=AcquireMagickResource(HeapResource,cache_info->length);
+          if ( heapNotMap != MagickFalse )
+            cache_info->pixels=(PixelPacket *) MagickAssumeAligned(
+              AcquireAlignedMemory(1,(size_t) cache_info->length));
+          else
+          {
+            cache_info->mapped=MagickTrue;
+            cache_info->pixels=(PixelPacket *) MapBlob(-1,IOMode,0,(size_t)
+              cache_info->length);
+          }
           if (cache_info->pixels == (PixelPacket *) NULL)
             cache_info->pixels=source_info.pixels;
           else
diff -ruN ImageMagick-6.9.4-8/magick/resource.c               magick/resource.c
--- ImageMagick-6.9.4-8/magick/resource.c                     2016-06-07 09:28:31.000000000 -0400
+++ magick/resource.c                                         2016-06-10 18:58:39.000000000 -0400
@@ -75,6 +75,7 @@
     width,
     height,
     area,
+    heap,
     memory,
     map,
     disk,
@@ -87,6 +88,7 @@
     width_limit,
     height_limit,
     area_limit,
+    heap_limit,
     memory_limit,
     map_limit,
     disk_limit,
@@ -108,6 +110,7 @@
     MagickULLConstant(0),              /* initial width */
     MagickULLConstant(0),              /* initial height */
     MagickULLConstant(0),              /* initial area */
+    MagickULLConstant(0),              /* initial heap */
     MagickULLConstant(0),              /* initial memory */
     MagickULLConstant(0),              /* initial map */
     MagickULLConstant(0),              /* initial disk */
@@ -118,6 +121,7 @@
     (INT_MAX/(5*sizeof(Quantum))),     /* width limit */
     (INT_MAX/(5*sizeof(Quantum))),     /* height limit */
     MagickULLConstant(3072)*1024*1024, /* area limit */
+    MagickULLConstant(3072)*1024*1024, /* heap limit */
     MagickULLConstant(1536)*1024*1024, /* memory limit */
     MagickULLConstant(3072)*1024*1024, /* map limit */
     MagickResourceInfinity,            /* disk limit */
@@ -217,6 +221,18 @@
         resource_limit);
       break;
     }
+    case HeapResource:
+    {
+      resource_info.heap=(MagickOffsetType) size;
+      limit=resource_info.heap_limit;
+      status=(resource_info.heap_limit == MagickResourceInfinity) ||
+        (size < limit) ? MagickTrue : MagickFalse;
+      (void) FormatMagickSize((MagickSizeType) resource_info.heap,MagickFalse,
+        resource_current);
+      (void) FormatMagickSize(resource_info.heap_limit,MagickFalse,
+        resource_limit);
+      break;
+    }
     case MemoryResource:
     {
       resource_info.memory+=size;
@@ -611,6 +627,11 @@
       resource=(MagickSizeType) resource_info.area;
       break;
     }
+    case HeapResource:
+    {
+      resource=(MagickSizeType) resource_info.heap;
+      break;
+    }
     case MemoryResource:
     {
       resource=(MagickSizeType) resource_info.memory;
@@ -701,6 +722,11 @@
       resource=resource_info.area_limit;
       break;
     }
+    case HeapResource:
+    {
+      resource=resource_info.heap_limit;
+      break;
+    }
     case MemoryResource:
     {
       resource=resource_info.memory_limit;
@@ -826,6 +852,7 @@
 {
   char
     area_limit[MaxTextExtent],
+    heap_limit[MaxTextExtent],
     disk_limit[MaxTextExtent],
     height_limit[MaxTextExtent],
     map_limit[MaxTextExtent],
@@ -843,6 +870,7 @@
   (void) FormatPixelSize(resource_info.width_limit,MagickFalse,width_limit);
   (void) FormatPixelSize(resource_info.height_limit,MagickFalse,height_limit);
   (void) FormatPixelSize(resource_info.area_limit,MagickFalse,area_limit);
+  (void) FormatPixelSize(resource_info.heap_limit,MagickFalse,heap_limit);
   (void) FormatMagickSize(resource_info.memory_limit,MagickTrue,memory_limit);
   (void) FormatMagickSize(resource_info.map_limit,MagickTrue,map_limit);
   (void) CopyMagickString(disk_limit,"unlimited",MaxTextExtent);
@@ -856,6 +884,7 @@
   (void) FormatLocaleFile(file,"  Width: %s\n",width_limit);
   (void) FormatLocaleFile(file,"  Height: %s\n",height_limit);
   (void) FormatLocaleFile(file,"  Area: %s\n",area_limit);
+  (void) FormatLocaleFile(file,"  Heap: %s\n",heap_limit);
   (void) FormatLocaleFile(file,"  Memory: %s\n",memory_limit);
   (void) FormatLocaleFile(file,"  Map: %s\n",map_limit);
   (void) FormatLocaleFile(file,"  Disk: %s\n",disk_limit);
@@ -937,6 +966,15 @@
         resource_limit);
       break;
     }
+    case HeapResource:
+    {
+      resource_info.heap=(MagickOffsetType) size;
+      (void) FormatMagickSize((MagickSizeType) resource_info.heap,MagickFalse,
+        resource_current);
+      (void) FormatMagickSize(resource_info.heap_limit,MagickFalse,
+        resource_limit);
+      break;
+    }
     case MemoryResource:
     {
       resource_info.memory-=size;
@@ -1137,6 +1175,13 @@
       (void) SetMagickResourceLimit(AreaResource,StringToSizeType(limit,100.0));
       limit=DestroyString(limit);
     }
+  (void) SetMagickResourceLimit(HeapResource,2*memory);
+  limit=GetEnvironmentValue("MAGICK_HEAP_LIMIT");
+  if (limit != (char *) NULL)
+    {
+      (void) SetMagickResourceLimit(HeapResource,StringToSizeType(limit,100.0));
+      limit=DestroyString(limit);
+    }
   (void) SetMagickResourceLimit(MemoryResource,memory);
   limit=GetEnvironmentValue("MAGICK_MEMORY_LIMIT");
   if (limit != (char *) NULL)
@@ -1308,6 +1353,14 @@
         resource_info.area_limit=MagickMin(limit,StringToSizeType(value,100.0));
       break;
     }
+    case HeapResource:
+    {
+      resource_info.heap_limit=limit;
+      value=GetPolicyValue("heap");
+      if (value != (char *) NULL)
+        resource_info.heap_limit=MagickMin(limit,StringToSizeType(value,100.0));
+      break;
+    }
     case MemoryResource:
     {
       resource_info.memory_limit=limit;
diff -ruN ImageMagick-6.9.4-8/Magick++/lib/ResourceLimits.cpp Magick++/lib/ResourceLimits.cpp
--- ImageMagick-6.9.4-8/Magick++/lib/ResourceLimits.cpp       2016-06-07 09:28:31.000000000 -0400
+++ Magick++/lib/ResourceLimits.cpp                           2016-06-10 18:58:39.000000000 -0400
@@ -20,6 +20,16 @@
   return GetMagickResourceLimit(AreaResource);
 }
 
+void Magick::ResourceLimits::heap(const MagickSizeType limit_)
+{
+  (void) SetMagickResourceLimit(HeapResource,limit_);
+}
+
+MagickCore::MagickSizeType Magick::ResourceLimits::heap(void)
+{
+  return GetMagickResourceLimit(HeapResource);
+}
+
 void Magick::ResourceLimits::disk(const MagickSizeType limit_)
 {
   (void) SetMagickResourceLimit(DiskResource,limit_);
diff -ruN ImageMagick-6.9.4-8/config/policy.xml               config/policy.xml
--- ImageMagick-6.9.4-8/config/policy.xml                     2016-06-07 09:28:31.000000000 -0400
+++ config/policy.xml                                         2016-06-10 18:58:39.000000000 -0400
@@ -43,7 +43,11 @@
 
     <policy domain="resource" name="area" value="1GB"/>
 
-  Define arguments for the memory, map, area, width, height, and disk resources
+  Any large memory image is cached to an anonymous memory map rather than heap memory:
+
+    <policy domain="resource" name="heap" value="64MB"/>
+
+  Define arguments for the memory, map, area, heap, width, height, and disk resources
   with SI prefixes (.e.g 100MB).  In addition, resource policies are maximums
   for each instance of ImageMagick (e.g. policy memory limit 1GB, -limit 2GB
   exceeds policy maximum so memory limit is 1GB).
@@ -55,6 +59,7 @@
   <!-- <policy domain="resource" name="width" value="10MP"/> -->
   <!-- <policy domain="resource" name="height" value="10MP"/> -->
   <!-- <policy domain="resource" name="area" value="1GB"/> -->
+  <!-- <policy domain="resource" name="heap" value="64MB"/> -->
   <!-- <policy domain="resource" name="disk" value="16EB"/> -->
   <!-- <policy domain="resource" name="file" value="768"/> -->
   <!-- <policy domain="resource" name="thread" value="4"/> -->
diff -ruN ImageMagick-6.9.4-8/utilities/import.1.in           utilities/import.1.in
--- ImageMagick-6.9.4-8/utilities/import.1.in                 2016-06-07 09:28:31.000000000 -0400
+++ utilities/import.1.in                                     2016-06-10 18:58:39.000000000 -0400
@@ -36,7 +36,7 @@
   \-interlace type      None, Line, Plane, or Partition
   \-interpolate method  pixel color interpolation method
   \-label string        assign a label to an image
-  \-limit type value    Area, Disk, Map, or Memory resource limit
+  \-limit type value    Area, Heap, Disk, Map, or Memory resource limit
   \-monitor             monitor progress
   \-page geometry       size and location of an image canvas
   \-pause seconds       seconds delay between snapshots
diff -ruN ImageMagick-6.9.4-8/www/architecture.html           www/architecture.html
--- ImageMagick-6.9.4-8/www/architecture.html                 2016-06-07 09:28:31.000000000 -0400
+++ www/architecture.html                                     2016-06-10 18:58:39.000000000 -0400
@@ -261,6 +261,8 @@
   <dd>maximum height of an image.  Exceed this limit and an exception is thrown and processing stops.</dd>
   <dt>area</dt>
   <dd>maximum area in bytes of any one image that can reside in the pixel cache memory.  If this limit is exceeded, the image is automagically cached to disk and optionally memory-mapped.</dd>
+  <dt>heap</dt>
+  <dd>maximum size in bytes of a pixel cache memory allocation that can reside in heap memory.  If this limit is exceeded, the image is automagically allocated with private anonymous mapping.</dd>
   <dt>memory</dt>
   <dd>maximum amount of memory in bytes to allocate for the pixel cache from the heap.</dd>
   <dt>map</dt>
@@ -283,6 +285,7 @@
   Width: 100MP
   Height: 100MP
   Area: 25.181GB
+  Heap: 512MiB
   Memory: 11.726GiB
   Map: 23.452GiB
   Disk: unlimited
@@ -301,6 +304,7 @@
   <policy domain="resource" name="width" value="8KP"/>
   <policy domain="resource" name="height" value="8KP"/>
   <policy domain="resource" name="area" value="128MB"/>
+  <policy domain="resource" name="heap" value="50MB"/>
   <policy domain="resource" name="disk" value="1GiB"/>
   <policy domain="resource" name="file" value="768"/>
   <policy domain="resource" name="thread" value="2"/>
diff -ruN ImageMagick-6.9.4-8/www/command-line-options.html   www/command-line-options.html
--- ImageMagick-6.9.4-8/www/command-line-options.html         2016-06-07 09:28:31.000000000 -0400
+++ www/command-line-options.html                             2016-06-10 18:58:39.000000000 -0400
@@ -5271,10 +5271,10 @@
 
 <p class="magick-description">Set the pixel cache resource limit.</p>
 
-<p>Choose from: <code>width</code>, <code>height</code>, <code>area</code>, <code>memory</code>, <code>map</code>, <code>disk</code>, <code>file</code>, <code>thread</code>,  <code>throttle</code>, or <code>time</code>.</p>
+<p>Choose from: <code>width</code>, <code>height</code>, <code>area</code>, <code>heap</code>, <code>memory</code>, <code>map</code>, <code>disk</code>, <code>file</code>, <code>thread</code>,  <code>throttle</code>, or <code>time</code>.</p>
 
 <p>The value for <code>file</code> is in number of files. The other limits are
-in bytes. Define arguments for the memory, map, area, and disk resource limits
+in bytes. Define arguments for the memory, map, area, heap, and disk resource limits
 with SI prefixes (.e.g 100MB).</p>
 
 <p>By default the limits are 768 files, 3GB of image area, 1.5GiB memory, 3GiB
@@ -5296,6 +5296,7 @@
   Width: 100MP
   Height: 100MP
   Area: 25.181GB
+  Heap: 512MiB
   Memory: 11.726GiB
   Map: 23.452GiB
   Disk: unlimited
@@ -5311,7 +5312,10 @@
 and whether the system honors a resource request. If the total size of
 allocated pixel storage in the given pool reaches the corresponding limit, the
 request is passed to the next pool. Additionally, requests that exceed the
-<code>area</code> limit automagically are allocated on disk.</p>
+<code>area</code> limit automagically are allocated on disk.
+Requests that exceed the <code>heap</code> limit automagically are allocated
+as an anonymous memory-mapping (memory in the VM buffer cache, but not backed
+by any file.).</p>
 
 <p>To illustrate how ImageMagick utilizes resource limits, consider a typical
 image resource request.  First, ImageMagick tries to allocate the pixels in
@@ -5353,13 +5357,24 @@
 
 <p>Here ImageMagick stops processing if an image requires more than 500MB of disk storage.</p>
 
+<p>In a long-running service daemon linked against the -lMagickCore shared library,
+you might have a concern about "malloc" heap fragmentation.  Because your service
+process might churn through huge numbers of large pixel cache allocations, you
+could set a limit on the size of "malloc" allocations, so that larger pixel
+cache blobs will be allocated as anonymous memory-map blobs, which will be stored
+in the VM buffer cache, but not backed by any file.</p>
+
+<pre>
+-limit area 500MiB -limit heap 50MiB
+</pre>
+
 <p>In addition to command-line resource limit option, resources can be set
 with <a href="resources.html#environment" >environment variables</a>. Set the
-environment variables <code>MAGICK_AREA_LIMIT</code>,
+environment variables <code>MAGICK_AREA_LIMIT</code>, <code>MAGICK_HEAP_LIMIT</code>,
 <code>MAGICK_DISK_LIMIT</code>, <code>MAGICK_FILE_LIMIT</code>,
 <code>MAGICK_MEMORY_LIMIT</code>, <code>MAGICK_MAP_LIMIT</code>,
 <code>MAGICK_THREAD_LIMIT</code>, <code>MAGICK_TIME_LIMIT</code> for limits of
-image area, disk space, open files, heap memory, memory map, number of threads
+image area, heap allocations, disk space, open files, private (heap or anonymous) memory, memory map, number of threads
 of execution, and maximum elapsed time in seconds respectively.</p>
 
 <p> Inquisitive users can try adding <a href="command-line-options.html#debug">-debug cache</a> to
diff -ruN ImageMagick-6.9.4-8/www/resources.html              www/resources.html
--- ImageMagick-6.9.4-8/www/resources.html                    2016-06-07 09:28:31.000000000 -0400
+++ www/resources.html                                        2016-06-10 18:58:39.000000000 -0400
@@ -131,6 +131,12 @@
 
 Any image larger than this area limit is cached to disk rather than memory.
 
+<pre>
+<policy domain="resource" name="heap" value="50MB"/>
+</pre>
+
+Any image larger than this byte limit (but smaller than the area limit) is cached in an anonymous memory map rather than heap memory.
+
 Use <code>width</code> to limit the maximum width of an image in pixels.  Exceed this limit and an exception is thrown and processing stops.
 
 <pre>
@@ -142,7 +148,7 @@
 <pre>
 <policy domain="resource" name="time" value="300"/>
 </pre>
-Define arguments for the memory, map, area, and disk resources with SI prefixes (.e.g 100MB).  In addition, resource policies are maximums for each instance of ImageMagick (e.g. policy memory limit 1GB, the <code>-limit 2GB</code> option exceeds policy maximum so memory limit is 1GB). </dd>
+Define arguments for the memory, map, area, heap, and disk resources with SI prefixes (.e.g 100MB).  In addition, resource policies are maximums for each instance of ImageMagick (e.g. policy memory limit 1GB, the <code>-limit 2GB</code> option exceeds policy maximum so memory limit is 1GB). </dd>
 
 <dt><a href="../source/quantization-table.xml">quantization-table.xml</a></dt>
   <dd>Custom JPEG quantization tables. Activate with <code>-define:q-table=quantization-table.xml</code>.</dd>
@@ -274,6 +280,10 @@
     <td>Set the maximum <var>width * height</var> of an image that can reside in the pixel cache memory.  Images that exceed the area limit are cached to disk (see <a href="resources.html#disk-limit">MAGICK_DISK_LIMIT</a>) and optionally memory-mapped.</td>
   </tr>
   <tr>
+    <td>MAGICK_HEAP_LIMIT</td>
+    <td>Set the maximum byte size of an image that can reside in the pixel cache as a heap memory allocation.  Images that exceed the byte limit are cached to an anonymous memory map blob, which is stored in the VM buffer cache, but not backed by any file.</td>
+  </tr>
+  <tr>
     <td>MAGICK_CODER_FILTER_PATH</td>
     <td>Set search path to use when searching for filter process modules (invoked via  <a href="command-line-options.html#process">-process</a>).  This path permits the user to extend ImageMagick's image processing functionality by adding loadable modules to a preferred location rather than copying them into the ImageMagick installation directory.  The formatting of the search path is similar to operating system search paths (i.e. colon delimited for Unix, and semi-colon delimited for Microsoft Windows). This user specified search path is searched before trying the <a href="resources.html#modules">default search path</a>.</td>
   </tr>
diff -ruN ImageMagick-6.9.4-8/www/source/policy.xml           www/source/policy.xml
--- ImageMagick-6.9.4-8/www/source/policy.xml                 2016-06-07 09:28:31.000000000 -0400
+++ www/source/policy.xml                                     2016-06-10 18:58:39.000000000 -0400
@@ -43,7 +43,11 @@
 
     <policy domain="resource" name="area" value="1GB"/>
 
-  Define arguments for the memory, map, area, width, height, and disk resources
+  Any large memory image is cached to an anonymous memory map rather than heap memory:
+
+    <policy domain="resource" name="heap" value="64MB"/>
+
+  Define arguments for the memory, map, area, heap, width, height, and disk resources
   with SI prefixes (.e.g 100MB).  In addition, resource policies are maximums
   for each instance of ImageMagick (e.g. policy memory limit 1GB, -limit 2GB
   exceeds policy maximum so memory limit is 1GB).
@@ -55,6 +59,7 @@
   <!-- <policy domain="resource" name="width" value="10MP"/> -->
   <!-- <policy domain="resource" name="height" value="10MP"/> -->
   <!-- <policy domain="resource" name="area" value="1GB"/> -->
+  <!-- <policy domain="resource" name="heap" value="64MB"/> -->
   <!-- <policy domain="resource" name="disk" value="16EB"/> -->
   <!-- <policy domain="resource" name="file" value="768"/> -->
   <!-- <policy domain="resource" name="thread" value="4"/> -->

[magick]

Many malloc()'s already use anonymous maps for large memory allocations. From the malloc() manual: "When allocating blocks of memory larger than MMAP_THRESHOLD bytes, the glibc malloc() implementation allocates the memory as a private anonymous mapping using mmap(2). MMAP_THRESHOLD is 128 kB by default, but is adjustable using mallopt(3)." Given this feature of malloc(), can you see any advantage to your patch other than with implementations of malloc() that do not utilize anonymous maps?

[dvdcrn]

Amazing. I never knew that, and didn't even think to look into malloc. I certainly could do that, and 128 kB isn't much bigger than I was using in my example. It makes me wonder what is growing my malloc heap, though. We're only processing about 3,000-6,000 photos per day these days, and only the smallest 80x80 (50KiB) would be under 128 KiB -- so I doubt it's heap fragmentation. Even a flat out leak of those 80x80 images would only cause RSS growth of 150-300 MiB per day; I am seeing much stronger growth -- about 1 GiB per day. That growth seems to have stopped with a tight java heap and a major GC about every two minutes (time will tell).

I do still think the patch makes sense. The ImageMagick-6.9.4-8 code really took very little patching to get it to mmap explicitly. It really seemed like the original author may have intended it, since debug logging at cache level even distinguishes between "Heap Memory" and "Anonymous Memory". The logging wouldn't pick up the distinction if malloc makes the choice. Making it explicit is nice, and it's a natural progression. I also see a clue in the documentation. The documentation through at least ImageMagick-6.9.2-3 also implied that anonymous memory maps were used for the Pixel Cache. It changed at this commit (http://git.imagemagick.org/repos/Websit ... 8%20commit). (We recently upgraded because of the security problems.)

In ImageMagick-6.9.2-3/www/architecture.html, documentation says this ...

Code: Select all

The pixel cache storage may be heap memory, anonymous memory mapped memory, disk-backed memory mapped, or on disk.

Starting at least with ImageMagick-6.9.4-1/www/architecture.html, documentation says this ...

Code: Select all

The pixel cache storage may be heap memory, disk-backed memory mapped, or on disk.

Really, seeing that documentation is why I started looking into the code.

[magick]

At some point we did support anonymous memory maps for the pixel cache, but removed them because malloc() does this already and whenever possible, we like to rely on system libraries-- that way if anything goes wrong we can blame the system library . You might have noticed that we do use anonymous maps for the pixel cache staging buffers known internally as 'nexus_info'. The thinking was that these are smaller allocations so they might not meet the 128k requirement of malloc() to utilize anonymous maps. We will rethink your suggestion over the next few weeks and implement if we can grok any advantages.

[dvdcrn]

It does make sense to just let malloc do the anonymous memory maps. I actually hesitate to make any adjustments, since I'm sure the JVM implementation uses malloc internally, and I have no idea what could go wrong. A couple suggestions to make this clearer:

* Explain the malloc behavior in the ImageMagick documentation somehow. What you wrote earlier was perfect. It actually explains why I can see RSS of my daemon spiking during processing of a large image.

* Perhaps you could expose the mallopt call through a similar pixel resource limit to the "heap" setting I proposed. The 128 KiB cutoff is probably well-suited for almost everything (the glibc folks do know what they're doing). Anyone who feels they must adjust it would find it difficult without changing the code.

[dvdcrn]

^^^ adjustments == adjustments (in "C" code in the jmagick implementation)

[magick]

You should be able to tweak the malloc behavior now with environment variables without the need of changes to the ImageMagick source distribution. For example, use MALLOC_MMAP_THRESHOLD_ to change the 128K default setting for when to use mmap() rather than sbrk.

[dvdcrn]

OK, that's cool. I'm actually fine with the 128KiB cutoff, and I'm a little afraid to mess with malloc in a JVM process, since we run so much on the same server. If it ain't broke.

I am now sure that constant major GC runs are preventing the problem. My process RSS "only" grew 200MiB in the past 2 days, and I was seeing 10X stronger growth last week without the constant major GC runs, even when I started the explicit MagickImage.destroyImages() calls. Since we restart this daemon process at least weekly, this shouldn't develop into a crisis. And we do watch these things.

Also, by "constant major GC runs", I really mean every GC run, which is happening about every 2 minutes. It's not much of a drag on throughput. I only increased the -Xmx a few weeks ago because I was taking a close look at this process when we upgraded ImageMagick. I thought to myself, "Wow, a major GC run every 2 minutes can't be healthy," so I put a stop to it by bumping -Xmx up 10%. I was younger and more naive then. Since then, I've fixed our bug by adding missing MagickImage.destroyImages() calls and learned a lot about ImageMagick internals and glibc malloc.

Isn't indeterminacy fun?