From 852124d49b447bbd5d4430c70dbbc02e05582b77 Mon Sep 17 00:00:00 2001 From: Alexander Date: Fri, 15 Sep 2017 00:09:36 +0300 Subject: [PATCH 01/27] Add params check for ImagingNewBlock --- libImaging/Storage.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/libImaging/Storage.c b/libImaging/Storage.c index 615c5fa2016..0b5e742decd 100644 --- a/libImaging/Storage.c +++ b/libImaging/Storage.c @@ -173,7 +173,6 @@ ImagingNewPrologueSubtype(const char *mode, int xsize, int ysize, int size) im->linesize = xsize * 4; } else if (strcmp(mode, "RGBa") == 0) { - /* EXPERIMENTAL */ /* 32-bit true colour images with premultiplied alpha */ im->bands = im->pixelsize = 4; im->linesize = xsize * 4; @@ -423,6 +422,10 @@ ImagingNewBlock(const char* mode, int xsize, int ysize) { Imaging im; + if (xsize < 0 || ysize < 0) { + return (Imaging) ImagingError_ValueError("bad image size"); + } + im = ImagingNewPrologue(mode, xsize, ysize); if ( ! im) return NULL; From 0a3c852e1b549a9f72a4f14762dece2d84375e24 Mon Sep 17 00:00:00 2001 From: Alexander Date: Fri, 15 Sep 2017 17:34:13 +0300 Subject: [PATCH 02/27] work in ImagingAllocateArray with blocks --- libImaging/Imaging.h | 3 +- libImaging/Storage.c | 90 +++++++++++++++++++++++++++++++++----------- 2 files changed, 71 insertions(+), 22 deletions(-) diff --git a/libImaging/Imaging.h b/libImaging/Imaging.h index a48d373eed6..d5b2a2a9dac 100644 --- a/libImaging/Imaging.h +++ b/libImaging/Imaging.h @@ -94,7 +94,8 @@ struct ImagingMemoryInstance { /* Internals */ char **image; /* Actual raster data. */ - char *block; /* Set if data is allocated in a single block. */ + char *block; /* Set if data is allocated in a single block. */ + char **blocks; /* Memory blocks for pixel storage */ int pixelsize; /* Size of a pixel, in bytes (1, 2 or 4) */ int linesize; /* Size of a line, in bytes (xsize * pixelsize) */ diff --git a/libImaging/Storage.c b/libImaging/Storage.c index 0b5e742decd..38287739915 100644 --- a/libImaging/Storage.c +++ b/libImaging/Storage.c @@ -264,44 +264,92 @@ ImagingDelete(Imaging im) /* ------------------ */ /* Allocate image as an array of line buffers. */ +#define MEMORY_BLOCK_SIZE (1024*1024) +#define MEMORY_BLOCKS_COUNT 128 + +void **_blocks = NULL; +int _blocks_free = 0; + +void * +_get_block(int dirty) +{ + void *block; + if ( ! _blocks) { + _blocks = calloc(sizeof(void*), MEMORY_BLOCKS_COUNT); + } + if (_blocks_free > 0) { + _blocks_free -= 1; + block = _blocks[_blocks_free]; + if ( ! dirty) { + memset(block, 0, MEMORY_BLOCK_SIZE); + } + } else { + if (dirty) { + block = malloc(MEMORY_BLOCK_SIZE); + } else { + block = calloc(1, MEMORY_BLOCK_SIZE); + } + } + return block; +} + +void +_return_block(void *block) +{ + free(block); +} + + static void ImagingDestroyArray(Imaging im) { - int y; + int y = 0; - if (im->image) - for (y = 0; y < im->ysize; y++) - if (im->image[y]) - free(im->image[y]); + if (im->blocks) { + while (im->blocks[y]) { + _return_block(im->blocks[y]); + y ++; + } + free(im->blocks); + } } Imaging ImagingAllocateArray(Imaging im, int dirty) { - ImagingSectionCookie cookie; - - int y; + int y, line_in_block, current_block; char* p; + int lines_pre_block, blocks_count; - ImagingSectionEnter(&cookie); + lines_pre_block = MEMORY_BLOCK_SIZE / im->linesize; + blocks_count = (im->ysize + lines_pre_block - 1) / lines_pre_block; + + /* One extra ponter is always NULL */ + im->blocks = (char **)calloc(sizeof(char *), blocks_count + 1); + if ( ! im->blocks) { + return (Imaging) ImagingError_MemoryError(); + } /* Allocate image as an array of lines */ - for (y = 0; y < im->ysize; y++) { - /* malloc check linesize checked in prologue */ - if (dirty) { - p = (char *) malloc(im->linesize); - } else { - p = (char *) calloc(1, im->linesize); + for (y = 0, line_in_block = 0, current_block = 0; y < im->ysize; y++) { + if (line_in_block == 0) { + p = (char *)_get_block(dirty); + if ( ! p) { + ImagingDestroyArray(im); + break; + } + im->blocks[current_block] = p; } - if (!p) { - ImagingDestroyArray(im); - break; + im->image[y] = p + im->linesize * line_in_block; + + line_in_block += 1; + if (line_in_block >= lines_pre_block) { + /* Reset counter and start new block */ + line_in_block = 0; + current_block += 1; } - im->image[y] = p; } - ImagingSectionLeave(&cookie); - if (y != im->ysize) { return (Imaging) ImagingError_MemoryError(); } From f584f8399a91c7cb0fa02c0bff58dab770d46198 Mon Sep 17 00:00:00 2001 From: Alexander Date: Fri, 15 Sep 2017 18:00:15 +0300 Subject: [PATCH 03/27] save released blocks --- libImaging/Storage.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/libImaging/Storage.c b/libImaging/Storage.c index 38287739915..d6c5773b87d 100644 --- a/libImaging/Storage.c +++ b/libImaging/Storage.c @@ -265,7 +265,7 @@ ImagingDelete(Imaging im) /* Allocate image as an array of line buffers. */ #define MEMORY_BLOCK_SIZE (1024*1024) -#define MEMORY_BLOCKS_COUNT 128 +#define MEMORY_MAX_BLOCKS 128 void **_blocks = NULL; int _blocks_free = 0; @@ -275,7 +275,7 @@ _get_block(int dirty) { void *block; if ( ! _blocks) { - _blocks = calloc(sizeof(void*), MEMORY_BLOCKS_COUNT); + _blocks = calloc(sizeof(void*), MEMORY_MAX_BLOCKS); } if (_blocks_free > 0) { _blocks_free -= 1; @@ -296,7 +296,12 @@ _get_block(int dirty) void _return_block(void *block) { - free(block); + if (_blocks_free < MEMORY_MAX_BLOCKS) { + _blocks[_blocks_free] = block; + _blocks_free += 1; + } else { + free(block); + } } @@ -308,7 +313,7 @@ ImagingDestroyArray(Imaging im) if (im->blocks) { while (im->blocks[y]) { _return_block(im->blocks[y]); - y ++; + y += 1; } free(im->blocks); } From fe283b10a503f62f9ac4d2b09c1fd9c6a3849d8c Mon Sep 17 00:00:00 2001 From: Alexander Date: Fri, 15 Sep 2017 18:11:20 +0300 Subject: [PATCH 04/27] Do not use ImagingNewBlock at all --- libImaging/Storage.c | 32 +++++--------------------------- 1 file changed, 5 insertions(+), 27 deletions(-) diff --git a/libImaging/Storage.c b/libImaging/Storage.c index d6c5773b87d..77cc3e97c8b 100644 --- a/libImaging/Storage.c +++ b/libImaging/Storage.c @@ -377,17 +377,13 @@ ImagingDestroyBlock(Imaging im) } Imaging -ImagingAllocateBlock(Imaging im, int dirty) +ImagingAllocateBlock(Imaging im) { Py_ssize_t y, i; - /* We shouldn't overflow, since the threshold defined - below says that we're only going to allocate max 4M - here before going to the array allocator. Check anyway. - */ + /* overflow check for malloc */ if (im->linesize && im->ysize > INT_MAX / im->linesize) { - /* punt if we're going to overflow */ return (Imaging) ImagingError_MemoryError(); } @@ -397,13 +393,8 @@ ImagingAllocateBlock(Imaging im, int dirty) platforms */ im->block = (char *) malloc(1); } else { - if (dirty) { - /* malloc check ok, overflow check above */ - im->block = (char *) malloc(im->ysize * im->linesize); - } else { - /* malloc check ok, overflow check above */ - im->block = (char *) calloc(im->ysize, im->linesize); - } + /* malloc check ok, overflow check above */ + im->block = (char *) calloc(im->ysize, im->linesize); } if ( ! im->block) { @@ -423,11 +414,6 @@ ImagingAllocateBlock(Imaging im, int dirty) /* -------------------------------------------------------------------- * Create a new, internally allocated, image. */ -#if defined(IMAGING_SMALL_MODEL) -#define THRESHOLD 16384L -#else -#define THRESHOLD (2048*2048*4L) -#endif Imaging ImagingNewInternal(const char* mode, int xsize, int ysize, int dirty) @@ -442,14 +428,6 @@ ImagingNewInternal(const char* mode, int xsize, int ysize, int dirty) if ( ! im) return NULL; - if (im->ysize && im->linesize <= THRESHOLD / im->ysize) { - if (ImagingAllocateBlock(im, dirty)) { - return im; - } - /* assume memory error; try allocating in array mode instead */ - ImagingError_Clear(); - } - if (ImagingAllocateArray(im, dirty)) { return im; } @@ -483,7 +461,7 @@ ImagingNewBlock(const char* mode, int xsize, int ysize) if ( ! im) return NULL; - if (ImagingAllocateBlock(im, 0)) { + if (ImagingAllocateBlock(im)) { return im; } From 883fb8f9e98c518636c6649f6b3f5802d13c14cb Mon Sep 17 00:00:00 2001 From: Alexander Date: Fri, 15 Sep 2017 19:00:53 +0300 Subject: [PATCH 05/27] MEMORY_MAX_BLOCKS should be 0 by default --- libImaging/Storage.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libImaging/Storage.c b/libImaging/Storage.c index 77cc3e97c8b..df874f2e5e1 100644 --- a/libImaging/Storage.c +++ b/libImaging/Storage.c @@ -265,7 +265,7 @@ ImagingDelete(Imaging im) /* Allocate image as an array of line buffers. */ #define MEMORY_BLOCK_SIZE (1024*1024) -#define MEMORY_MAX_BLOCKS 128 +#define MEMORY_MAX_BLOCKS 0 void **_blocks = NULL; int _blocks_free = 0; From d4a1f7a01a34a5d3f436ee351f7940632586ce2d Mon Sep 17 00:00:00 2001 From: Alexander Date: Sat, 16 Sep 2017 19:00:46 +0300 Subject: [PATCH 06/27] align lines --- libImaging/Storage.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/libImaging/Storage.c b/libImaging/Storage.c index df874f2e5e1..d2e0328f5e0 100644 --- a/libImaging/Storage.c +++ b/libImaging/Storage.c @@ -266,6 +266,7 @@ ImagingDelete(Imaging im) #define MEMORY_BLOCK_SIZE (1024*1024) #define MEMORY_MAX_BLOCKS 0 +#define MEMORY_ALIGN_LINES 64 void **_blocks = NULL; int _blocks_free = 0; @@ -324,9 +325,10 @@ ImagingAllocateArray(Imaging im, int dirty) { int y, line_in_block, current_block; char* p; - int lines_pre_block, blocks_count; + int lines_pre_block, blocks_count, linesize; - lines_pre_block = MEMORY_BLOCK_SIZE / im->linesize; + linesize = ((im->linesize - 1) / MEMORY_ALIGN_LINES + 1) * MEMORY_ALIGN_LINES; + lines_pre_block = MEMORY_BLOCK_SIZE / linesize; blocks_count = (im->ysize + lines_pre_block - 1) / lines_pre_block; /* One extra ponter is always NULL */ @@ -345,7 +347,7 @@ ImagingAllocateArray(Imaging im, int dirty) } im->blocks[current_block] = p; } - im->image[y] = p + im->linesize * line_in_block; + im->image[y] = p + linesize * line_in_block; line_in_block += 1; if (line_in_block >= lines_pre_block) { From f2123b42226cbd0264bbeecd36a4b0ce69b555f5 Mon Sep 17 00:00:00 2001 From: Alexander Date: Sun, 17 Sep 2017 01:22:44 +0300 Subject: [PATCH 07/27] limit allocated memory to lines_per_block * linesize size allocate block for wider lines --- libImaging/Storage.c | 38 +++++++++++++++++++++++--------------- 1 file changed, 23 insertions(+), 15 deletions(-) diff --git a/libImaging/Storage.c b/libImaging/Storage.c index d2e0328f5e0..57d46c41813 100644 --- a/libImaging/Storage.c +++ b/libImaging/Storage.c @@ -265,30 +265,34 @@ ImagingDelete(Imaging im) /* Allocate image as an array of line buffers. */ #define MEMORY_BLOCK_SIZE (1024*1024) -#define MEMORY_MAX_BLOCKS 0 -#define MEMORY_ALIGN_LINES 64 +#define MEMORY_CACHE_BLOCKS 0 +#define MEMORY_ALIGN_LINES 1 void **_blocks = NULL; int _blocks_free = 0; void * -_get_block(int dirty) +_get_block(int requested_size, int dirty) { void *block; if ( ! _blocks) { - _blocks = calloc(sizeof(void*), MEMORY_MAX_BLOCKS); + _blocks = calloc(sizeof(void*), MEMORY_CACHE_BLOCKS); } if (_blocks_free > 0) { _blocks_free -= 1; - block = _blocks[_blocks_free]; + block = realloc(_blocks[_blocks_free], requested_size); + if ( ! block) { + free(_blocks[_blocks_free]); + return NULL; + } if ( ! dirty) { - memset(block, 0, MEMORY_BLOCK_SIZE); + memset(block, 0, requested_size); } } else { if (dirty) { - block = malloc(MEMORY_BLOCK_SIZE); + block = malloc(requested_size); } else { - block = calloc(1, MEMORY_BLOCK_SIZE); + block = calloc(1, requested_size); } } return block; @@ -297,7 +301,7 @@ _get_block(int dirty) void _return_block(void *block) { - if (_blocks_free < MEMORY_MAX_BLOCKS) { + if (_blocks_free < MEMORY_CACHE_BLOCKS) { _blocks[_blocks_free] = block; _blocks_free += 1; } else { @@ -325,11 +329,14 @@ ImagingAllocateArray(Imaging im, int dirty) { int y, line_in_block, current_block; char* p; - int lines_pre_block, blocks_count, linesize; + int linesize, lines_per_block, blocks_count; + + linesize = (im->linesize + MEMORY_ALIGN_LINES - 1) & -MEMORY_ALIGN_LINES; + lines_per_block = MEMORY_BLOCK_SIZE / linesize; + if (lines_per_block <= 0) + lines_per_block = 1; + blocks_count = (im->ysize + lines_per_block - 1) / lines_per_block; - linesize = ((im->linesize - 1) / MEMORY_ALIGN_LINES + 1) * MEMORY_ALIGN_LINES; - lines_pre_block = MEMORY_BLOCK_SIZE / linesize; - blocks_count = (im->ysize + lines_pre_block - 1) / lines_pre_block; /* One extra ponter is always NULL */ im->blocks = (char **)calloc(sizeof(char *), blocks_count + 1); @@ -340,17 +347,18 @@ ImagingAllocateArray(Imaging im, int dirty) /* Allocate image as an array of lines */ for (y = 0, line_in_block = 0, current_block = 0; y < im->ysize; y++) { if (line_in_block == 0) { - p = (char *)_get_block(dirty); + p = (char *)_get_block(lines_per_block * linesize, dirty); if ( ! p) { ImagingDestroyArray(im); break; } im->blocks[current_block] = p; } + im->image[y] = p + linesize * line_in_block; line_in_block += 1; - if (line_in_block >= lines_pre_block) { + if (line_in_block >= lines_per_block) { /* Reset counter and start new block */ line_in_block = 0; current_block += 1; From a5034b54cdd9dd58e4673388da569b0bff4f32d8 Mon Sep 17 00:00:00 2001 From: Alexander Date: Sun, 17 Sep 2017 01:34:57 +0300 Subject: [PATCH 08/27] do not request more lines than required --- libImaging/Storage.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/libImaging/Storage.c b/libImaging/Storage.c index 57d46c41813..d39720d5705 100644 --- a/libImaging/Storage.c +++ b/libImaging/Storage.c @@ -337,7 +337,6 @@ ImagingAllocateArray(Imaging im, int dirty) lines_per_block = 1; blocks_count = (im->ysize + lines_per_block - 1) / lines_per_block; - /* One extra ponter is always NULL */ im->blocks = (char **)calloc(sizeof(char *), blocks_count + 1); if ( ! im->blocks) { @@ -347,7 +346,11 @@ ImagingAllocateArray(Imaging im, int dirty) /* Allocate image as an array of lines */ for (y = 0, line_in_block = 0, current_block = 0; y < im->ysize; y++) { if (line_in_block == 0) { - p = (char *)_get_block(lines_per_block * linesize, dirty); + int lines_remained = lines_per_block; + if (lines_remained > im->ysize - y) { + lines_remained = im->ysize - y; + } + p = (char *)_get_block(lines_remained * linesize, dirty); if ( ! p) { ImagingDestroyArray(im); break; From 6007e818a9d4afe0cb3bc74920133d012ccdea2f Mon Sep 17 00:00:00 2001 From: Alexander Date: Sun, 17 Sep 2017 01:40:30 +0300 Subject: [PATCH 09/27] debug messages --- libImaging/Storage.c | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/libImaging/Storage.c b/libImaging/Storage.c index d39720d5705..63fc27f23fb 100644 --- a/libImaging/Storage.c +++ b/libImaging/Storage.c @@ -280,11 +280,16 @@ _get_block(int requested_size, int dirty) } if (_blocks_free > 0) { _blocks_free -= 1; + // printf("get block: %p %d; _blocks_free: %d\n", + // _blocks[_blocks_free], requested_size, _blocks_free); block = realloc(_blocks[_blocks_free], requested_size); if ( ! block) { free(_blocks[_blocks_free]); return NULL; } + // if (block != _blocks[_blocks_free]) { + // printf("reallocat: %p %p\n", block, _blocks[_blocks_free]); + // } if ( ! dirty) { memset(block, 0, requested_size); } @@ -294,6 +299,8 @@ _get_block(int requested_size, int dirty) } else { block = calloc(1, requested_size); } + // printf("allocated: %p %d; _blocks_free: %d\n", + // block, requested_size, _blocks_free); } return block; } @@ -304,7 +311,9 @@ _return_block(void *block) if (_blocks_free < MEMORY_CACHE_BLOCKS) { _blocks[_blocks_free] = block; _blocks_free += 1; + // printf("ret block: %p; _blocks_free: %d\n", block, _blocks_free); } else { + // printf("fre block: %p; _blocks_free: %d\n", block, _blocks_free); free(block); } } @@ -336,6 +345,8 @@ ImagingAllocateArray(Imaging im, int dirty) if (lines_per_block <= 0) lines_per_block = 1; blocks_count = (im->ysize + lines_per_block - 1) / lines_per_block; + // printf("NEW size: %dx%d, ls: %d, lpb: %d, blocks: %d\n", + // im->xsize, im->ysize, linesize, lines_per_block, blocks_count); /* One extra ponter is always NULL */ im->blocks = (char **)calloc(sizeof(char *), blocks_count + 1); From 00547431006396e9925fd6f51eb0ce1de45620cf Mon Sep 17 00:00:00 2001 From: Alexander Date: Sun, 17 Sep 2017 02:58:01 +0300 Subject: [PATCH 10/27] fix zero size images --- Tests/test_image.py | 6 ++++++ libImaging/Storage.c | 9 +++++++-- 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/Tests/test_image.py b/Tests/test_image.py index 0c104f0aa35..1209c992003 100644 --- a/Tests/test_image.py +++ b/Tests/test_image.py @@ -385,6 +385,12 @@ def test_check_size(self): im = Image.new('L', (0, 0)) self.assertEqual(im.size, (0, 0)) + im = Image.new('L', (0, 100)) + self.assertEqual(im.size, (0, 100)) + + im = Image.new('L', (100, 0)) + self.assertEqual(im.size, (100, 0)) + self.assertTrue(Image.new('RGB', (1, 1))) # Should pass lists too i = Image.new('RGB', [1, 1]) diff --git a/libImaging/Storage.c b/libImaging/Storage.c index 63fc27f23fb..45fd0f71dac 100644 --- a/libImaging/Storage.c +++ b/libImaging/Storage.c @@ -278,6 +278,9 @@ _get_block(int requested_size, int dirty) if ( ! _blocks) { _blocks = calloc(sizeof(void*), MEMORY_CACHE_BLOCKS); } + if ( ! requested_size) { + requested_size = 1; + } if (_blocks_free > 0) { _blocks_free -= 1; // printf("get block: %p %d; _blocks_free: %d\n", @@ -341,9 +344,11 @@ ImagingAllocateArray(Imaging im, int dirty) int linesize, lines_per_block, blocks_count; linesize = (im->linesize + MEMORY_ALIGN_LINES - 1) & -MEMORY_ALIGN_LINES; - lines_per_block = MEMORY_BLOCK_SIZE / linesize; - if (lines_per_block <= 0) + if ( ! linesize || linesize > MEMORY_BLOCK_SIZE) { lines_per_block = 1; + } else { + lines_per_block = MEMORY_BLOCK_SIZE / linesize; + } blocks_count = (im->ysize + lines_per_block - 1) / lines_per_block; // printf("NEW size: %dx%d, ls: %d, lpb: %d, blocks: %d\n", // im->xsize, im->ysize, linesize, lines_per_block, blocks_count); From fd907fbdc9d3fc17b924c279d8fb7b2faeb4e58c Mon Sep 17 00:00:00 2001 From: Alexander Date: Sun, 17 Sep 2017 03:31:24 +0300 Subject: [PATCH 11/27] Fix 0-width and 0-height images other way --- libImaging/Storage.c | 24 +++++++++--------------- 1 file changed, 9 insertions(+), 15 deletions(-) diff --git a/libImaging/Storage.c b/libImaging/Storage.c index 45fd0f71dac..9f9d8998a11 100644 --- a/libImaging/Storage.c +++ b/libImaging/Storage.c @@ -278,9 +278,6 @@ _get_block(int requested_size, int dirty) if ( ! _blocks) { _blocks = calloc(sizeof(void*), MEMORY_CACHE_BLOCKS); } - if ( ! requested_size) { - requested_size = 1; - } if (_blocks_free > 0) { _blocks_free -= 1; // printf("get block: %p %d; _blocks_free: %d\n", @@ -343,16 +340,20 @@ ImagingAllocateArray(Imaging im, int dirty) char* p; int linesize, lines_per_block, blocks_count; + /* 0-width or 0-height image. No need to do anything */ + if ( ! im->linesize || ! im->ysize) { + return im; + } + linesize = (im->linesize + MEMORY_ALIGN_LINES - 1) & -MEMORY_ALIGN_LINES; - if ( ! linesize || linesize > MEMORY_BLOCK_SIZE) { + lines_per_block = MEMORY_BLOCK_SIZE / linesize; + if (lines_per_block == 0) lines_per_block = 1; - } else { - lines_per_block = MEMORY_BLOCK_SIZE / linesize; - } blocks_count = (im->ysize + lines_per_block - 1) / lines_per_block; // printf("NEW size: %dx%d, ls: %d, lpb: %d, blocks: %d\n", // im->xsize, im->ysize, linesize, lines_per_block, blocks_count); + im->destroy = ImagingDestroyArray; /* One extra ponter is always NULL */ im->blocks = (char **)calloc(sizeof(char *), blocks_count + 1); if ( ! im->blocks) { @@ -368,8 +369,7 @@ ImagingAllocateArray(Imaging im, int dirty) } p = (char *)_get_block(lines_remained * linesize, dirty); if ( ! p) { - ImagingDestroyArray(im); - break; + return (Imaging) ImagingError_MemoryError(); } im->blocks[current_block] = p; } @@ -384,12 +384,6 @@ ImagingAllocateArray(Imaging im, int dirty) } } - if (y != im->ysize) { - return (Imaging) ImagingError_MemoryError(); - } - - im->destroy = ImagingDestroyArray; - return im; } From c8a2923d175a0b2bc9f7c7bbe875bfd19b1305f9 Mon Sep 17 00:00:00 2001 From: Alexander Date: Sun, 17 Sep 2017 19:54:49 +0300 Subject: [PATCH 12/27] Return blocks in reverse order to reduce reallocations --- libImaging/Storage.c | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/libImaging/Storage.c b/libImaging/Storage.c index 9f9d8998a11..506126044a4 100644 --- a/libImaging/Storage.c +++ b/libImaging/Storage.c @@ -287,9 +287,9 @@ _get_block(int requested_size, int dirty) free(_blocks[_blocks_free]); return NULL; } - // if (block != _blocks[_blocks_free]) { - // printf("reallocat: %p %p\n", block, _blocks[_blocks_free]); - // } + if (block != _blocks[_blocks_free]) { + // printf("reallocat: %p %p\n", block, _blocks[_blocks_free]); + } if ( ! dirty) { memset(block, 0, requested_size); } @@ -361,7 +361,10 @@ ImagingAllocateArray(Imaging im, int dirty) } /* Allocate image as an array of lines */ - for (y = 0, line_in_block = 0, current_block = 0; y < im->ysize; y++) { + line_in_block = 0; + /* Return blocks in reverse order to reduce reallocations */ + current_block = blocks_count - 1; + for (y = 0; y < im->ysize; y++) { if (line_in_block == 0) { int lines_remained = lines_per_block; if (lines_remained > im->ysize - y) { @@ -380,7 +383,7 @@ ImagingAllocateArray(Imaging im, int dirty) if (line_in_block >= lines_per_block) { /* Reset counter and start new block */ line_in_block = 0; - current_block += 1; + current_block -= 1; } } From dc192be83f8dd8421d072dd50093ded97f81e2b7 Mon Sep 17 00:00:00 2001 From: Alexander Date: Sun, 17 Sep 2017 20:10:31 +0300 Subject: [PATCH 13/27] temp --- libImaging/Storage.c | 24 ++++++++++++++++++------ 1 file changed, 18 insertions(+), 6 deletions(-) diff --git a/libImaging/Storage.c b/libImaging/Storage.c index 506126044a4..f1f08df5fd2 100644 --- a/libImaging/Storage.c +++ b/libImaging/Storage.c @@ -268,11 +268,23 @@ ImagingDelete(Imaging im) #define MEMORY_CACHE_BLOCKS 0 #define MEMORY_ALIGN_LINES 1 -void **_blocks = NULL; -int _blocks_free = 0; +typedef struct { + int alignment = 1; + int block_size = 1024*1024; + int blocks_max = 0; + int blocks_free = 0; + void **blocks = NULL +} *ImagingMemoryArean; + + +void +memory_set_blocks_max(ImagingMemoryArean arena, int blocks_max) +{ + arena->blocks_max = blocks_max; +} void * -_get_block(int requested_size, int dirty) +memory_get_block(ImagingMemoryArean arena, int requested_size, int dirty) { void *block; if ( ! _blocks) { @@ -306,7 +318,7 @@ _get_block(int requested_size, int dirty) } void -_return_block(void *block) +memory_return_block(ImagingMemoryArean arena, void *block) { if (_blocks_free < MEMORY_CACHE_BLOCKS) { _blocks[_blocks_free] = block; @@ -345,7 +357,7 @@ ImagingAllocateArray(Imaging im, int dirty) return im; } - linesize = (im->linesize + MEMORY_ALIGN_LINES - 1) & -MEMORY_ALIGN_LINES; + linesize = (im->linesize + arena->alignment - 1) & -arena->alignment; lines_per_block = MEMORY_BLOCK_SIZE / linesize; if (lines_per_block == 0) lines_per_block = 1; @@ -370,7 +382,7 @@ ImagingAllocateArray(Imaging im, int dirty) if (lines_remained > im->ysize - y) { lines_remained = im->ysize - y; } - p = (char *)_get_block(lines_remained * linesize, dirty); + p = (char *)memory_get_block(lines_remained * linesize, dirty); if ( ! p) { return (Imaging) ImagingError_MemoryError(); } From 4951962af1924bf45d3644b8378071a4c282d1dc Mon Sep 17 00:00:00 2001 From: Alexander Date: Sun, 17 Sep 2017 20:31:13 +0300 Subject: [PATCH 14/27] ImagingMemoryArean tata type --- libImaging/Storage.c | 68 ++++++++++++++++++++++++-------------------- 1 file changed, 37 insertions(+), 31 deletions(-) diff --git a/libImaging/Storage.c b/libImaging/Storage.c index f1f08df5fd2..e01968234ee 100644 --- a/libImaging/Storage.c +++ b/libImaging/Storage.c @@ -264,22 +264,30 @@ ImagingDelete(Imaging im) /* ------------------ */ /* Allocate image as an array of line buffers. */ -#define MEMORY_BLOCK_SIZE (1024*1024) -#define MEMORY_CACHE_BLOCKS 0 -#define MEMORY_ALIGN_LINES 1 - -typedef struct { - int alignment = 1; - int block_size = 1024*1024; - int blocks_max = 0; - int blocks_free = 0; - void **blocks = NULL +typedef struct ImagingMemoryArean { + int alignment; + int block_size; + int blocks_max; + int blocks_free; + void **blocks; } *ImagingMemoryArean; +struct ImagingMemoryArean default_arena = { + .alignment = 1, + .block_size = 1024*1024, + .blocks_max = 0, + .blocks_free = 0, + .blocks = NULL +}; void memory_set_blocks_max(ImagingMemoryArean arena, int blocks_max) { + /* Free already cached blocks */ + while (arena->blocks_free > blocks_max) { + arena->blocks_free -= 1; + free(arena->blocks[arena->blocks_free]); + } arena->blocks_max = blocks_max; } @@ -287,20 +295,17 @@ void * memory_get_block(ImagingMemoryArean arena, int requested_size, int dirty) { void *block; - if ( ! _blocks) { - _blocks = calloc(sizeof(void*), MEMORY_CACHE_BLOCKS); - } - if (_blocks_free > 0) { - _blocks_free -= 1; - // printf("get block: %p %d; _blocks_free: %d\n", - // _blocks[_blocks_free], requested_size, _blocks_free); - block = realloc(_blocks[_blocks_free], requested_size); + if (arena->blocks_free > 0) { + arena->blocks_free -= 1; + // printf("get block: %p %d; free: %d\n", + // arena->blocks[arena->blocks_free], requested_size, arena->blocks_free); + block = realloc(arena->blocks[arena->blocks_free], requested_size); if ( ! block) { - free(_blocks[_blocks_free]); + free(arena->blocks[arena->blocks_free]); return NULL; } - if (block != _blocks[_blocks_free]) { - // printf("reallocat: %p %p\n", block, _blocks[_blocks_free]); + if (block != arena->blocks[arena->blocks_free]) { + // printf("reallocat: %p %p\n", block, arena->blocks[arena->blocks_free]); } if ( ! dirty) { memset(block, 0, requested_size); @@ -311,8 +316,8 @@ memory_get_block(ImagingMemoryArean arena, int requested_size, int dirty) } else { block = calloc(1, requested_size); } - // printf("allocated: %p %d; _blocks_free: %d\n", - // block, requested_size, _blocks_free); + // printf("allocated: %p %d; arena->blocks_free: %d\n", + // block, requested_size, arena->blocks_free); } return block; } @@ -320,12 +325,12 @@ memory_get_block(ImagingMemoryArean arena, int requested_size, int dirty) void memory_return_block(ImagingMemoryArean arena, void *block) { - if (_blocks_free < MEMORY_CACHE_BLOCKS) { - _blocks[_blocks_free] = block; - _blocks_free += 1; - // printf("ret block: %p; _blocks_free: %d\n", block, _blocks_free); + if (arena->blocks_free < arena->blocks_max) { + arena->blocks[arena->blocks_free] = block; + arena->blocks_free += 1; + // printf("ret block: %p; free: %d\n", block, arena->blocks_free); } else { - // printf("fre block: %p; _blocks_free: %d\n", block, _blocks_free); + // printf("fre block: %p; free: %d\n", block, arena->blocks_free); free(block); } } @@ -338,7 +343,7 @@ ImagingDestroyArray(Imaging im) if (im->blocks) { while (im->blocks[y]) { - _return_block(im->blocks[y]); + memory_return_block(&default_arena, im->blocks[y]); y += 1; } free(im->blocks); @@ -349,6 +354,7 @@ Imaging ImagingAllocateArray(Imaging im, int dirty) { int y, line_in_block, current_block; + ImagingMemoryArean arena = &default_arena; char* p; int linesize, lines_per_block, blocks_count; @@ -358,7 +364,7 @@ ImagingAllocateArray(Imaging im, int dirty) } linesize = (im->linesize + arena->alignment - 1) & -arena->alignment; - lines_per_block = MEMORY_BLOCK_SIZE / linesize; + lines_per_block = arena->block_size / linesize; if (lines_per_block == 0) lines_per_block = 1; blocks_count = (im->ysize + lines_per_block - 1) / lines_per_block; @@ -382,7 +388,7 @@ ImagingAllocateArray(Imaging im, int dirty) if (lines_remained > im->ysize - y) { lines_remained = im->ysize - y; } - p = (char *)memory_get_block(lines_remained * linesize, dirty); + p = memory_get_block(arena, lines_remained * linesize, dirty); if ( ! p) { return (Imaging) ImagingError_MemoryError(); } From 8659cd7564bfafb153a16a6dcf2e7aaa9da5e55d Mon Sep 17 00:00:00 2001 From: Alexander Date: Sun, 17 Sep 2017 21:31:50 +0300 Subject: [PATCH 15/27] stats --- _imaging.c | 2 +- libImaging/Imaging.h | 15 +++++++++++++- libImaging/Storage.c | 48 +++++++++++++++++++++++--------------------- 3 files changed, 40 insertions(+), 25 deletions(-) diff --git a/_imaging.c b/_imaging.c index d0777c73a70..8bcc6afd22e 100644 --- a/_imaging.c +++ b/_imaging.c @@ -647,7 +647,7 @@ _getcount(PyObject* self, PyObject* args) if (!PyArg_ParseTuple(args, ":getcount")) return NULL; - return PyInt_FromLong(ImagingNewCount); + return PyInt_FromLong(ImagingDefaultArena.stats_new_count); } static PyObject* diff --git a/libImaging/Imaging.h b/libImaging/Imaging.h index 62f8d12255f..4f202b2a770 100644 --- a/libImaging/Imaging.h +++ b/libImaging/Imaging.h @@ -153,11 +153,24 @@ struct ImagingPaletteInstance { }; +typedef struct ImagingMemoryArena { + int alignment; + int block_size; + int blocks_max; + int blocks_free; + void **blocks; + int stats_new_count; + int stats_allocated_block; + int stats_reused_block; + int stats_reallocated_block; + int stats_freed_block; +} *ImagingMemoryArena; + /* Objects */ /* ------- */ -extern int ImagingNewCount; +extern struct ImagingMemoryArena ImagingDefaultArena; extern Imaging ImagingNew(const char* mode, int xsize, int ysize); extern Imaging ImagingNewDirty(const char* mode, int xsize, int ysize); diff --git a/libImaging/Storage.c b/libImaging/Storage.c index e01968234ee..88043ad7af7 100644 --- a/libImaging/Storage.c +++ b/libImaging/Storage.c @@ -229,7 +229,7 @@ ImagingNewPrologueSubtype(const char *mode, int xsize, int ysize, int size) break; } - ImagingNewCount++; + ImagingDefaultArena.stats_new_count += 1; return im; } @@ -264,48 +264,52 @@ ImagingDelete(Imaging im) /* ------------------ */ /* Allocate image as an array of line buffers. */ -typedef struct ImagingMemoryArean { - int alignment; - int block_size; - int blocks_max; - int blocks_free; - void **blocks; -} *ImagingMemoryArean; - -struct ImagingMemoryArean default_arena = { +struct ImagingMemoryArena ImagingDefaultArena = { .alignment = 1, .block_size = 1024*1024, .blocks_max = 0, .blocks_free = 0, - .blocks = NULL + .blocks = NULL, + // Stats + 0, 0, 0, 0, 0 }; void -memory_set_blocks_max(ImagingMemoryArean arena, int blocks_max) +memory_set_blocks_max(ImagingMemoryArena arena, int blocks_max) { /* Free already cached blocks */ while (arena->blocks_free > blocks_max) { arena->blocks_free -= 1; free(arena->blocks[arena->blocks_free]); + arena->stats_freed_block += 1; } + arena->blocks_max = blocks_max; + if (blocks_max == 0 && arena->blocks != NULL) { + free(arena->blocks); + arena->blocks = NULL; + } else if (arena->blocks != NULL) { + arena->blocks = realloc(arena->blocks, sizeof(void*) * blocks_max); + } else { + arena->blocks = calloc(sizeof(void*), blocks_max); + } } void * -memory_get_block(ImagingMemoryArean arena, int requested_size, int dirty) +memory_get_block(ImagingMemoryArena arena, int requested_size, int dirty) { void *block; if (arena->blocks_free > 0) { arena->blocks_free -= 1; - // printf("get block: %p %d; free: %d\n", - // arena->blocks[arena->blocks_free], requested_size, arena->blocks_free); block = realloc(arena->blocks[arena->blocks_free], requested_size); if ( ! block) { free(arena->blocks[arena->blocks_free]); + arena->stats_freed_block += 1; return NULL; } + arena->stats_reused_block += 1; if (block != arena->blocks[arena->blocks_free]) { - // printf("reallocat: %p %p\n", block, arena->blocks[arena->blocks_free]); + arena->stats_reallocated_block += 1; } if ( ! dirty) { memset(block, 0, requested_size); @@ -316,22 +320,20 @@ memory_get_block(ImagingMemoryArean arena, int requested_size, int dirty) } else { block = calloc(1, requested_size); } - // printf("allocated: %p %d; arena->blocks_free: %d\n", - // block, requested_size, arena->blocks_free); + arena->stats_allocated_block += 1; } return block; } void -memory_return_block(ImagingMemoryArean arena, void *block) +memory_return_block(ImagingMemoryArena arena, void *block) { if (arena->blocks_free < arena->blocks_max) { arena->blocks[arena->blocks_free] = block; arena->blocks_free += 1; - // printf("ret block: %p; free: %d\n", block, arena->blocks_free); } else { - // printf("fre block: %p; free: %d\n", block, arena->blocks_free); free(block); + arena->stats_freed_block += 1; } } @@ -343,7 +345,7 @@ ImagingDestroyArray(Imaging im) if (im->blocks) { while (im->blocks[y]) { - memory_return_block(&default_arena, im->blocks[y]); + memory_return_block(&ImagingDefaultArena, im->blocks[y]); y += 1; } free(im->blocks); @@ -354,7 +356,7 @@ Imaging ImagingAllocateArray(Imaging im, int dirty) { int y, line_in_block, current_block; - ImagingMemoryArean arena = &default_arena; + ImagingMemoryArena arena = &ImagingDefaultArena; char* p; int linesize, lines_per_block, blocks_count; From 53dde3b7f69532b3130dc3a5efad4719a8052daa Mon Sep 17 00:00:00 2001 From: Alexander Date: Sun, 17 Sep 2017 22:46:11 +0300 Subject: [PATCH 16/27] fix visual c compiler --- libImaging/Storage.c | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/libImaging/Storage.c b/libImaging/Storage.c index 88043ad7af7..b8704edf535 100644 --- a/libImaging/Storage.c +++ b/libImaging/Storage.c @@ -265,13 +265,12 @@ ImagingDelete(Imaging im) /* Allocate image as an array of line buffers. */ struct ImagingMemoryArena ImagingDefaultArena = { - .alignment = 1, - .block_size = 1024*1024, - .blocks_max = 0, - .blocks_free = 0, - .blocks = NULL, - // Stats - 0, 0, 0, 0, 0 + 1, // alignment + 1*1024*1024, // block_size + 0, // blocks_max + 0, // blocks_free + NULL, // blocks + 0, 0, 0, 0, 0 // Stats }; void From af3dcf84aff560a7b0d45170d409725cbae6624f Mon Sep 17 00:00:00 2001 From: Alexander Date: Mon, 18 Sep 2017 01:41:39 +0300 Subject: [PATCH 17/27] python api for resources --- Tests/test_image_resample.py | 4 +- _imaging.c | 186 ++++++++++++++++++++++++++++++++--- docs/releasenotes/4.3.0.rst | 3 + libImaging/Imaging.h | 10 +- libImaging/Storage.c | 43 ++++++-- 5 files changed, 219 insertions(+), 27 deletions(-) diff --git a/Tests/test_image_resample.py b/Tests/test_image_resample.py index 600a52320c0..ee5a062c652 100644 --- a/Tests/test_image_resample.py +++ b/Tests/test_image_resample.py @@ -305,9 +305,9 @@ def test_dirty_pixels_la(self): class CoreResamplePassesTest(PillowTestCase): @contextmanager def count(self, diff): - count = Image.core.getcount() + count = Image.core.get_stats()['new_count'] yield - self.assertEqual(Image.core.getcount() - count, diff) + self.assertEqual(Image.core.get_stats()['new_count'] - count, diff) def test_horizontal(self): im = hopper('L') diff --git a/_imaging.c b/_imaging.c index 8bcc6afd22e..b6a5e8d0045 100644 --- a/_imaging.c +++ b/_imaging.c @@ -641,15 +641,6 @@ _new_block(PyObject* self, PyObject* args) return PyImagingNew(ImagingNewBlock(mode, xsize, ysize)); } -static PyObject* -_getcount(PyObject* self, PyObject* args) -{ - if (!PyArg_ParseTuple(args, ":getcount")) - return NULL; - - return PyInt_FromLong(ImagingDefaultArena.stats_new_count); -} - static PyObject* _linear_gradient(PyObject* self, PyObject* args) { @@ -3337,6 +3328,169 @@ static PyTypeObject PixelAccess_Type = { /* -------------------------------------------------------------------- */ +static PyObject* +_get_stats(PyObject* self, PyObject* args) +{ + PyObject* d; + ImagingMemoryArena arena = &ImagingDefaultArena; + + if (!PyArg_ParseTuple(args, ":get_stats")) + return NULL; + + d = PyDict_New(); + if ( ! d) + return NULL; + PyDict_SetItemString(d, "new_count", + PyInt_FromLong(arena->stats_new_count)); + PyDict_SetItemString(d, "allocated_blocks", + PyInt_FromLong(arena->stats_allocated_blocks)); + PyDict_SetItemString(d, "reused_blocks", + PyInt_FromLong(arena->stats_reused_blocks)); + PyDict_SetItemString(d, "reallocated_blocks", + PyInt_FromLong(arena->stats_reallocated_blocks)); + PyDict_SetItemString(d, "freed_blocks", + PyInt_FromLong(arena->stats_freed_blocks)); + return d; +} + +static PyObject* +_reset_stats(PyObject* self, PyObject* args) +{ + ImagingMemoryArena arena = &ImagingDefaultArena; + + if (!PyArg_ParseTuple(args, ":reset_stats")) + return NULL; + + arena->stats_new_count = 0; + arena->stats_allocated_blocks = 0; + arena->stats_reused_blocks = 0; + arena->stats_reallocated_blocks = 0; + arena->stats_freed_blocks = 0; + + Py_INCREF(Py_None); + return Py_None; +} + +static PyObject* +_get_alignment(PyObject* self, PyObject* args) +{ + if (!PyArg_ParseTuple(args, ":get_alignment")) + return NULL; + + return PyInt_FromLong(ImagingDefaultArena.alignment); +} + +static PyObject* +_get_block_size(PyObject* self, PyObject* args) +{ + if (!PyArg_ParseTuple(args, ":get_block_size")) + return NULL; + + return PyInt_FromLong(ImagingDefaultArena.block_size); +} + +static PyObject* +_get_blocks_max(PyObject* self, PyObject* args) +{ + if (!PyArg_ParseTuple(args, ":get_blocks_max")) + return NULL; + + return PyInt_FromLong(ImagingDefaultArena.blocks_max); +} + +static PyObject* +_get_blocks_free(PyObject* self, PyObject* args) +{ + if (!PyArg_ParseTuple(args, ":get_blocks_free")) + return NULL; + + return PyInt_FromLong(ImagingDefaultArena.blocks_free); +} + +static PyObject* +_set_alignment(PyObject* self, PyObject* args) +{ + int alignment; + if (!PyArg_ParseTuple(args, "i:set_alignment", &alignment)) + return NULL; + + if (alignment < 1 || alignment > 128) { + PyErr_SetString(PyExc_ValueError, "alignment should be from 1 to 128"); + return NULL; + } + /* Is power of two */ + if (alignment & (alignment - 1)) { + PyErr_SetString(PyExc_ValueError, "alignment should be power of two"); + return NULL; + } + + ImagingDefaultArena.alignment = alignment; + + Py_INCREF(Py_None); + return Py_None; +} + +static PyObject* +_set_block_size(PyObject* self, PyObject* args) +{ + int block_size; + if (!PyArg_ParseTuple(args, "i:set_block_size", &block_size)) + return NULL; + + if (block_size < 0) { + PyErr_SetString(PyExc_ValueError, + "block_size should be greater than 0"); + return NULL; + } + + if (block_size & 0xfff) { + PyErr_SetString(PyExc_ValueError, + "block_size should be multiple of 4096"); + return NULL; + } + + ImagingDefaultArena.block_size = block_size; + + Py_INCREF(Py_None); + return Py_None; +} + +static PyObject* +_set_blocks_max(PyObject* self, PyObject* args) +{ + int blocks_max; + if (!PyArg_ParseTuple(args, "i:set_blocks_max", &blocks_max)) + return NULL; + + if (blocks_max < 0) { + PyErr_SetString(PyExc_ValueError, + "blocks_max should be greater than 0"); + return NULL; + } + + if ( ! ImagingMemorySetBlocksMax(&ImagingDefaultArena, blocks_max)) { + ImagingError_MemoryError(); + return NULL; + } + + Py_INCREF(Py_None); + return Py_None; +} + +static PyObject* +_clear_cache(PyObject* self, PyObject* args) +{ + if (!PyArg_ParseTuple(args, ":_clear_cache")) + return NULL; + + ImagingMemoryClearCache(&ImagingDefaultArena); + + Py_INCREF(Py_None); + return Py_None; +} + +/* -------------------------------------------------------------------- */ + /* FIXME: this is something of a mess. Should replace this with pluggable codecs, but not before PIL 1.2 */ @@ -3400,8 +3554,6 @@ static PyMethodDef functions[] = { {"new", (PyCFunction)_new, 1}, {"merge", (PyCFunction)_merge, 1}, - {"getcount", (PyCFunction)_getcount, 1}, - /* Functions */ {"convert", (PyCFunction)_convert2, 1}, @@ -3491,6 +3643,18 @@ static PyMethodDef functions[] = { {"outline", (PyCFunction)PyOutline_Create, 1}, #endif + /* Resource management */ + {"get_stats", (PyCFunction)_get_stats, 1}, + {"reset_stats", (PyCFunction)_reset_stats, 1}, + {"get_alignment", (PyCFunction)_get_alignment, 1}, + {"get_block_size", (PyCFunction)_get_block_size, 1}, + {"get_blocks_max", (PyCFunction)_get_blocks_max, 1}, + {"get_blocks_free", (PyCFunction)_get_blocks_free, 1}, + {"set_alignment", (PyCFunction)_set_alignment, 1}, + {"set_block_size", (PyCFunction)_set_block_size, 1}, + {"set_blocks_max", (PyCFunction)_set_blocks_max, 1}, + {"clear_cache", (PyCFunction)_clear_cache, 1}, + {NULL, NULL} /* sentinel */ }; diff --git a/docs/releasenotes/4.3.0.rst b/docs/releasenotes/4.3.0.rst index 4708eeb2961..b893a623a81 100644 --- a/docs/releasenotes/4.3.0.rst +++ b/docs/releasenotes/4.3.0.rst @@ -52,3 +52,6 @@ identified the format of the clipboard data. The ``PIL.Image.core.copy`` and ``PIL.Image.Image.im.copy2`` methods have been removed. + +The ``PIL.Image.core.getcount`` methods have been removed, use +``PIL.Image.core.get_stats()['new_count']`` property instead. diff --git a/libImaging/Imaging.h b/libImaging/Imaging.h index 4f202b2a770..991808ca40b 100644 --- a/libImaging/Imaging.h +++ b/libImaging/Imaging.h @@ -160,10 +160,10 @@ typedef struct ImagingMemoryArena { int blocks_free; void **blocks; int stats_new_count; - int stats_allocated_block; - int stats_reused_block; - int stats_reallocated_block; - int stats_freed_block; + int stats_allocated_blocks; + int stats_reused_blocks; + int stats_reallocated_blocks; + int stats_freed_blocks; } *ImagingMemoryArena; @@ -171,6 +171,8 @@ typedef struct ImagingMemoryArena { /* ------- */ extern struct ImagingMemoryArena ImagingDefaultArena; +extern int ImagingMemorySetBlocksMax(ImagingMemoryArena arena, int blocks_max); +extern void ImagingMemoryClearCache(ImagingMemoryArena arena); extern Imaging ImagingNew(const char* mode, int xsize, int ysize); extern Imaging ImagingNewDirty(const char* mode, int xsize, int ysize); diff --git a/libImaging/Storage.c b/libImaging/Storage.c index b8704edf535..565adc8a1cd 100644 --- a/libImaging/Storage.c +++ b/libImaging/Storage.c @@ -273,24 +273,47 @@ struct ImagingMemoryArena ImagingDefaultArena = { 0, 0, 0, 0, 0 // Stats }; -void -memory_set_blocks_max(ImagingMemoryArena arena, int blocks_max) +int +ImagingMemorySetBlocksMax(ImagingMemoryArena arena, int blocks_max) { + void *p; /* Free already cached blocks */ while (arena->blocks_free > blocks_max) { arena->blocks_free -= 1; free(arena->blocks[arena->blocks_free]); - arena->stats_freed_block += 1; + arena->stats_freed_blocks += 1; } - arena->blocks_max = blocks_max; if (blocks_max == 0 && arena->blocks != NULL) { free(arena->blocks); arena->blocks = NULL; } else if (arena->blocks != NULL) { - arena->blocks = realloc(arena->blocks, sizeof(void*) * blocks_max); + p = realloc(arena->blocks, sizeof(void*) * blocks_max); + if ( ! p) { + // Leave previous blocks_max value + return 0; + } + arena->blocks = p; } else { arena->blocks = calloc(sizeof(void*), blocks_max); + if ( ! arena->blocks) { + // Fallback to 0 + arena->blocks_max = 0; + return 0; + } + } + arena->blocks_max = blocks_max; + + return 1; +} + +void +ImagingMemoryClearCache(ImagingMemoryArena arena) +{ + while (arena->blocks_free > 0) { + arena->blocks_free -= 1; + free(arena->blocks[arena->blocks_free]); + arena->stats_freed_blocks += 1; } } @@ -303,12 +326,12 @@ memory_get_block(ImagingMemoryArena arena, int requested_size, int dirty) block = realloc(arena->blocks[arena->blocks_free], requested_size); if ( ! block) { free(arena->blocks[arena->blocks_free]); - arena->stats_freed_block += 1; + arena->stats_freed_blocks += 1; return NULL; } - arena->stats_reused_block += 1; + arena->stats_reused_blocks += 1; if (block != arena->blocks[arena->blocks_free]) { - arena->stats_reallocated_block += 1; + arena->stats_reallocated_blocks += 1; } if ( ! dirty) { memset(block, 0, requested_size); @@ -319,7 +342,7 @@ memory_get_block(ImagingMemoryArena arena, int requested_size, int dirty) } else { block = calloc(1, requested_size); } - arena->stats_allocated_block += 1; + arena->stats_allocated_blocks += 1; } return block; } @@ -332,7 +355,7 @@ memory_return_block(ImagingMemoryArena arena, void *block) arena->blocks_free += 1; } else { free(block); - arena->stats_freed_block += 1; + arena->stats_freed_blocks += 1; } } From 6d2be876c8267f196d2c8701b5997471519a7837 Mon Sep 17 00:00:00 2001 From: Alexander Date: Mon, 18 Sep 2017 02:37:47 +0300 Subject: [PATCH 18/27] tests --- Tests/test_core_resources.py | 130 +++++++++++++++++++++++++++++++++++ _imaging.c | 14 +--- libImaging/Imaging.h | 2 +- libImaging/Storage.c | 30 ++++---- 4 files changed, 149 insertions(+), 27 deletions(-) create mode 100644 Tests/test_core_resources.py diff --git a/Tests/test_core_resources.py b/Tests/test_core_resources.py new file mode 100644 index 00000000000..67f9681e343 --- /dev/null +++ b/Tests/test_core_resources.py @@ -0,0 +1,130 @@ +from __future__ import division, print_function + +from helper import unittest, PillowTestCase +from PIL import Image + + +class TestCoreStats(PillowTestCase): + def test_get_stats(self): + # Create at least one image + Image.new('RGB', (10, 10)) + + stats = Image.core.get_stats() + self.assertIn('new_count', stats) + self.assertIn('reused_blocks', stats) + self.assertIn('freed_blocks', stats) + self.assertIn('allocated_blocks', stats) + self.assertIn('reallocated_blocks', stats) + self.assertIn('blocks_cached', stats) + + def test_reset_stats(self): + Image.core.reset_stats() + + stats = Image.core.get_stats() + self.assertEqual(stats['new_count'], 0) + self.assertEqual(stats['reused_blocks'], 0) + self.assertEqual(stats['freed_blocks'], 0) + self.assertEqual(stats['allocated_blocks'], 0) + self.assertEqual(stats['reallocated_blocks'], 0) + self.assertEqual(stats['blocks_cached'], 0) + + +class TestCoreMemory(PillowTestCase): + def tearDown(self): + # Restore default values + Image.core.set_alignment(1) + Image.core.set_block_size(1024*1024) + Image.core.set_blocks_max(0) + Image.core.clear_cache() + + def test_get_alignment(self): + alignment = Image.core.get_alignment() + + self.assertGreater(alignment, 0) + + def test_set_alignment(self): + for i in [1, 2, 4, 8, 16, 32]: + Image.core.set_alignment(i) + alignment = Image.core.get_alignment() + self.assertEqual(alignment, i) + + # Try to construct new image + Image.new('RGB', (10, 10)) + + self.assertRaises(ValueError, Image.core.set_alignment, 0) + self.assertRaises(ValueError, Image.core.set_alignment, -1) + self.assertRaises(ValueError, Image.core.set_alignment, 3) + + def test_get_block_size(self): + block_size = Image.core.get_block_size() + + self.assertGreaterEqual(block_size, 4096) + + def test_set_block_size(self): + for i in [4096, 2*4096, 3*4096]: + Image.core.set_block_size(i) + block_size = Image.core.get_block_size() + self.assertEqual(block_size, i) + + # Try to construct new image + Image.new('RGB', (10, 10)) + + self.assertRaises(ValueError, Image.core.set_block_size, 0) + self.assertRaises(ValueError, Image.core.set_block_size, -1) + self.assertRaises(ValueError, Image.core.set_block_size, 4000) + + def test_set_block_size_stats(self): + Image.core.reset_stats() + Image.core.set_blocks_max(0) + Image.core.set_block_size(4096) + Image.new('RGB', (256, 256)) + + stats = Image.core.get_stats() + self.assertGreaterEqual(stats['new_count'], 1) + self.assertGreaterEqual(stats['allocated_blocks'], 64) + self.assertGreaterEqual(stats['freed_blocks'], 64) + + def test_get_blocks_max(self): + blocks_max = Image.core.get_blocks_max() + + self.assertGreaterEqual(blocks_max, 0) + + def test_set_blocks_max(self): + for i in [0, 1, 10]: + Image.core.set_blocks_max(i) + blocks_max = Image.core.get_blocks_max() + self.assertEqual(blocks_max, i) + + # Try to construct new image + Image.new('RGB', (10, 10)) + + self.assertRaises(ValueError, Image.core.set_blocks_max, -1) + + def test_set_blocks_max_stats(self): + Image.core.reset_stats() + Image.core.set_blocks_max(128) + Image.core.set_block_size(4096) + Image.new('RGB', (256, 256)) + Image.new('RGB', (256, 256)) + + stats = Image.core.get_stats() + self.assertGreaterEqual(stats['new_count'], 2) + self.assertGreaterEqual(stats['allocated_blocks'], 64) + self.assertGreaterEqual(stats['reused_blocks'], 64) + self.assertEqual(stats['freed_blocks'], 0) + self.assertEqual(stats['blocks_cached'], 64) + + def test_clear_cache_stats(self): + Image.core.reset_stats() + Image.core.set_blocks_max(128) + Image.core.set_block_size(4096) + Image.new('RGB', (256, 256)) + Image.new('RGB', (256, 256)) + Image.core.clear_cache() + + stats = Image.core.get_stats() + self.assertGreaterEqual(stats['new_count'], 2) + self.assertGreaterEqual(stats['allocated_blocks'], 64) + self.assertGreaterEqual(stats['reused_blocks'], 64) + self.assertGreaterEqual(stats['freed_blocks'], 64) + self.assertEqual(stats['blocks_cached'], 0) diff --git a/_imaging.c b/_imaging.c index b6a5e8d0045..1fe10226f60 100644 --- a/_imaging.c +++ b/_imaging.c @@ -3350,6 +3350,8 @@ _get_stats(PyObject* self, PyObject* args) PyInt_FromLong(arena->stats_reallocated_blocks)); PyDict_SetItemString(d, "freed_blocks", PyInt_FromLong(arena->stats_freed_blocks)); + PyDict_SetItemString(d, "blocks_cached", + PyInt_FromLong(arena->blocks_cached)); return d; } @@ -3398,15 +3400,6 @@ _get_blocks_max(PyObject* self, PyObject* args) return PyInt_FromLong(ImagingDefaultArena.blocks_max); } -static PyObject* -_get_blocks_free(PyObject* self, PyObject* args) -{ - if (!PyArg_ParseTuple(args, ":get_blocks_free")) - return NULL; - - return PyInt_FromLong(ImagingDefaultArena.blocks_free); -} - static PyObject* _set_alignment(PyObject* self, PyObject* args) { @@ -3437,7 +3430,7 @@ _set_block_size(PyObject* self, PyObject* args) if (!PyArg_ParseTuple(args, "i:set_block_size", &block_size)) return NULL; - if (block_size < 0) { + if (block_size <= 0) { PyErr_SetString(PyExc_ValueError, "block_size should be greater than 0"); return NULL; @@ -3649,7 +3642,6 @@ static PyMethodDef functions[] = { {"get_alignment", (PyCFunction)_get_alignment, 1}, {"get_block_size", (PyCFunction)_get_block_size, 1}, {"get_blocks_max", (PyCFunction)_get_blocks_max, 1}, - {"get_blocks_free", (PyCFunction)_get_blocks_free, 1}, {"set_alignment", (PyCFunction)_set_alignment, 1}, {"set_block_size", (PyCFunction)_set_block_size, 1}, {"set_blocks_max", (PyCFunction)_set_blocks_max, 1}, diff --git a/libImaging/Imaging.h b/libImaging/Imaging.h index 991808ca40b..3cc815c4053 100644 --- a/libImaging/Imaging.h +++ b/libImaging/Imaging.h @@ -157,7 +157,7 @@ typedef struct ImagingMemoryArena { int alignment; int block_size; int blocks_max; - int blocks_free; + int blocks_cached; void **blocks; int stats_new_count; int stats_allocated_blocks; diff --git a/libImaging/Storage.c b/libImaging/Storage.c index 565adc8a1cd..363c60dbffc 100644 --- a/libImaging/Storage.c +++ b/libImaging/Storage.c @@ -268,7 +268,7 @@ struct ImagingMemoryArena ImagingDefaultArena = { 1, // alignment 1*1024*1024, // block_size 0, // blocks_max - 0, // blocks_free + 0, // blocks_cached NULL, // blocks 0, 0, 0, 0, 0 // Stats }; @@ -278,9 +278,9 @@ ImagingMemorySetBlocksMax(ImagingMemoryArena arena, int blocks_max) { void *p; /* Free already cached blocks */ - while (arena->blocks_free > blocks_max) { - arena->blocks_free -= 1; - free(arena->blocks[arena->blocks_free]); + while (arena->blocks_cached > blocks_max) { + arena->blocks_cached -= 1; + free(arena->blocks[arena->blocks_cached]); arena->stats_freed_blocks += 1; } @@ -310,9 +310,9 @@ ImagingMemorySetBlocksMax(ImagingMemoryArena arena, int blocks_max) void ImagingMemoryClearCache(ImagingMemoryArena arena) { - while (arena->blocks_free > 0) { - arena->blocks_free -= 1; - free(arena->blocks[arena->blocks_free]); + while (arena->blocks_cached > 0) { + arena->blocks_cached -= 1; + free(arena->blocks[arena->blocks_cached]); arena->stats_freed_blocks += 1; } } @@ -321,16 +321,16 @@ void * memory_get_block(ImagingMemoryArena arena, int requested_size, int dirty) { void *block; - if (arena->blocks_free > 0) { - arena->blocks_free -= 1; - block = realloc(arena->blocks[arena->blocks_free], requested_size); + if (arena->blocks_cached > 0) { + arena->blocks_cached -= 1; + block = realloc(arena->blocks[arena->blocks_cached], requested_size); if ( ! block) { - free(arena->blocks[arena->blocks_free]); + free(arena->blocks[arena->blocks_cached]); arena->stats_freed_blocks += 1; return NULL; } arena->stats_reused_blocks += 1; - if (block != arena->blocks[arena->blocks_free]) { + if (block != arena->blocks[arena->blocks_cached]) { arena->stats_reallocated_blocks += 1; } if ( ! dirty) { @@ -350,9 +350,9 @@ memory_get_block(ImagingMemoryArena arena, int requested_size, int dirty) void memory_return_block(ImagingMemoryArena arena, void *block) { - if (arena->blocks_free < arena->blocks_max) { - arena->blocks[arena->blocks_free] = block; - arena->blocks_free += 1; + if (arena->blocks_cached < arena->blocks_max) { + arena->blocks[arena->blocks_cached] = block; + arena->blocks_cached += 1; } else { free(block); arena->stats_freed_blocks += 1; From ae104b0d0ea895f7b8f3f89125984d0b88419421 Mon Sep 17 00:00:00 2001 From: Alexander Date: Mon, 18 Sep 2017 02:51:27 +0300 Subject: [PATCH 19/27] collect garbage before check memory --- Tests/test_core_resources.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/Tests/test_core_resources.py b/Tests/test_core_resources.py index 67f9681e343..a55075990e1 100644 --- a/Tests/test_core_resources.py +++ b/Tests/test_core_resources.py @@ -1,5 +1,7 @@ from __future__ import division, print_function +import gc + from helper import unittest, PillowTestCase from PIL import Image @@ -106,6 +108,7 @@ def test_set_blocks_max_stats(self): Image.core.set_block_size(4096) Image.new('RGB', (256, 256)) Image.new('RGB', (256, 256)) + gc.collect() stats = Image.core.get_stats() self.assertGreaterEqual(stats['new_count'], 2) @@ -120,6 +123,7 @@ def test_clear_cache_stats(self): Image.core.set_block_size(4096) Image.new('RGB', (256, 256)) Image.new('RGB', (256, 256)) + gc.collect() Image.core.clear_cache() stats = Image.core.get_stats() From 23527774d376abccbbbbd5eda1a3490cee86c4f0 Mon Sep 17 00:00:00 2001 From: Alexander Date: Mon, 18 Sep 2017 02:57:14 +0300 Subject: [PATCH 20/27] test for images wider than block_size --- Tests/test_core_resources.py | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/Tests/test_core_resources.py b/Tests/test_core_resources.py index a55075990e1..5c25d7545aa 100644 --- a/Tests/test_core_resources.py +++ b/Tests/test_core_resources.py @@ -132,3 +132,18 @@ def test_clear_cache_stats(self): self.assertGreaterEqual(stats['reused_blocks'], 64) self.assertGreaterEqual(stats['freed_blocks'], 64) self.assertEqual(stats['blocks_cached'], 0) + + def test_large_images(self): + Image.core.reset_stats() + Image.core.set_blocks_max(0) + Image.core.set_block_size(4096) + Image.new('RGB', (2048, 16)) + gc.collect() + Image.core.clear_cache() + + stats = Image.core.get_stats() + self.assertGreaterEqual(stats['new_count'], 1) + self.assertGreaterEqual(stats['allocated_blocks'], 16) + self.assertGreaterEqual(stats['reused_blocks'], 0) + self.assertGreaterEqual(stats['freed_blocks'], 16) + self.assertEqual(stats['blocks_cached'], 0) From 2ab19bbe445a5670289a951f1c408d31e5733e52 Mon Sep 17 00:00:00 2001 From: Alexander Date: Mon, 18 Sep 2017 03:17:45 +0300 Subject: [PATCH 21/27] actually fix tests on pypy --- Tests/test_core_resources.py | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/Tests/test_core_resources.py b/Tests/test_core_resources.py index 5c25d7545aa..46ca12e386d 100644 --- a/Tests/test_core_resources.py +++ b/Tests/test_core_resources.py @@ -1,11 +1,14 @@ from __future__ import division, print_function -import gc +import sys from helper import unittest, PillowTestCase from PIL import Image +is_pypy = hasattr(sys, 'pypy_version_info') + + class TestCoreStats(PillowTestCase): def test_get_stats(self): # Create at least one image @@ -84,7 +87,8 @@ def test_set_block_size_stats(self): stats = Image.core.get_stats() self.assertGreaterEqual(stats['new_count'], 1) self.assertGreaterEqual(stats['allocated_blocks'], 64) - self.assertGreaterEqual(stats['freed_blocks'], 64) + if not is_pypy: + self.assertGreaterEqual(stats['freed_blocks'], 64) def test_get_blocks_max(self): blocks_max = Image.core.get_blocks_max() @@ -102,13 +106,13 @@ def test_set_blocks_max(self): self.assertRaises(ValueError, Image.core.set_blocks_max, -1) + @unittest.skipIf(is_pypy, "images are not collected") def test_set_blocks_max_stats(self): Image.core.reset_stats() Image.core.set_blocks_max(128) Image.core.set_block_size(4096) Image.new('RGB', (256, 256)) Image.new('RGB', (256, 256)) - gc.collect() stats = Image.core.get_stats() self.assertGreaterEqual(stats['new_count'], 2) @@ -117,13 +121,13 @@ def test_set_blocks_max_stats(self): self.assertEqual(stats['freed_blocks'], 0) self.assertEqual(stats['blocks_cached'], 64) + @unittest.skipIf(is_pypy, "images are not collected") def test_clear_cache_stats(self): Image.core.reset_stats() Image.core.set_blocks_max(128) Image.core.set_block_size(4096) Image.new('RGB', (256, 256)) Image.new('RGB', (256, 256)) - gc.collect() Image.core.clear_cache() stats = Image.core.get_stats() @@ -138,12 +142,12 @@ def test_large_images(self): Image.core.set_blocks_max(0) Image.core.set_block_size(4096) Image.new('RGB', (2048, 16)) - gc.collect() Image.core.clear_cache() stats = Image.core.get_stats() self.assertGreaterEqual(stats['new_count'], 1) self.assertGreaterEqual(stats['allocated_blocks'], 16) self.assertGreaterEqual(stats['reused_blocks'], 0) - self.assertGreaterEqual(stats['freed_blocks'], 16) self.assertEqual(stats['blocks_cached'], 0) + if not is_pypy: + self.assertGreaterEqual(stats['freed_blocks'], 16) From 3bfdfcd48a03705bf7512c3454c418ec13db2d86 Mon Sep 17 00:00:00 2001 From: Alexander Date: Mon, 18 Sep 2017 03:57:43 +0300 Subject: [PATCH 22/27] reduce code duplication --- _imaging.c | 2 +- libImaging/Imaging.h | 2 +- libImaging/Storage.c | 12 +++--------- 3 files changed, 5 insertions(+), 11 deletions(-) diff --git a/_imaging.c b/_imaging.c index 1fe10226f60..ddbc303f87e 100644 --- a/_imaging.c +++ b/_imaging.c @@ -3476,7 +3476,7 @@ _clear_cache(PyObject* self, PyObject* args) if (!PyArg_ParseTuple(args, ":_clear_cache")) return NULL; - ImagingMemoryClearCache(&ImagingDefaultArena); + ImagingMemoryClearCache(&ImagingDefaultArena, 0); Py_INCREF(Py_None); return Py_None; diff --git a/libImaging/Imaging.h b/libImaging/Imaging.h index 3cc815c4053..14fac0225d0 100644 --- a/libImaging/Imaging.h +++ b/libImaging/Imaging.h @@ -172,7 +172,7 @@ typedef struct ImagingMemoryArena { extern struct ImagingMemoryArena ImagingDefaultArena; extern int ImagingMemorySetBlocksMax(ImagingMemoryArena arena, int blocks_max); -extern void ImagingMemoryClearCache(ImagingMemoryArena arena); +extern void ImagingMemoryClearCache(ImagingMemoryArena arena, int new_size); extern Imaging ImagingNew(const char* mode, int xsize, int ysize); extern Imaging ImagingNewDirty(const char* mode, int xsize, int ysize); diff --git a/libImaging/Storage.c b/libImaging/Storage.c index 363c60dbffc..edd4ffe1480 100644 --- a/libImaging/Storage.c +++ b/libImaging/Storage.c @@ -278,11 +278,7 @@ ImagingMemorySetBlocksMax(ImagingMemoryArena arena, int blocks_max) { void *p; /* Free already cached blocks */ - while (arena->blocks_cached > blocks_max) { - arena->blocks_cached -= 1; - free(arena->blocks[arena->blocks_cached]); - arena->stats_freed_blocks += 1; - } + ImagingMemoryClearCache(arena, blocks_max); if (blocks_max == 0 && arena->blocks != NULL) { free(arena->blocks); @@ -297,8 +293,6 @@ ImagingMemorySetBlocksMax(ImagingMemoryArena arena, int blocks_max) } else { arena->blocks = calloc(sizeof(void*), blocks_max); if ( ! arena->blocks) { - // Fallback to 0 - arena->blocks_max = 0; return 0; } } @@ -308,9 +302,9 @@ ImagingMemorySetBlocksMax(ImagingMemoryArena arena, int blocks_max) } void -ImagingMemoryClearCache(ImagingMemoryArena arena) +ImagingMemoryClearCache(ImagingMemoryArena arena, int new_size) { - while (arena->blocks_cached > 0) { + while (arena->blocks_cached > new_size) { arena->blocks_cached -= 1; free(arena->blocks[arena->blocks_cached]); arena->stats_freed_blocks += 1; From 67e1e03c79072a022f4aa9930cf048b598f9ad0c Mon Sep 17 00:00:00 2001 From: Alexander Date: Mon, 18 Sep 2017 21:29:48 +0300 Subject: [PATCH 23/27] Set options from environment variables --- PIL/Image.py | 37 +++++++++++++++++++++++++++++++++++++ libImaging/Storage.c | 2 +- 2 files changed, 38 insertions(+), 1 deletion(-) diff --git a/PIL/Image.py b/PIL/Image.py index e49b1555d9c..a10bb7eaea6 100644 --- a/PIL/Image.py +++ b/PIL/Image.py @@ -2824,3 +2824,40 @@ def radial_gradient(mode): :param mode: Input mode. """ return Image()._new(core.radial_gradient(mode)) + + +# -------------------------------------------------------------------- +# Resources + +def _apply_env_variables(env=None): + if env is None: + env = os.environ + + for var_name, setter in [ + ('PILLOW_ALIGNMENT', core.set_alignment), + ('PILLOW_BLOCK_SIZE', core.set_block_size), + ('PILLOW_BLOCKS_MAX', core.set_blocks_max), + ]: + if var_name not in env: + continue + + var = env[var_name].lower() + + units = 1 + for postfix, mul in [('k', 1024), ('m', 1024*1024)]: + if var.endswith(postfix): + units = mul + var = var[:-len(postfix)] + + try: + var = int(var) * units + except ValueError: + warnings.warn("{0} is not int".format(var_name)) + continue + + try: + setter(var) + except ValueError as e: + warnings.warn("{0}: {1}".format(var_name, e)) + +_apply_env_variables() diff --git a/libImaging/Storage.c b/libImaging/Storage.c index edd4ffe1480..433e100cf95 100644 --- a/libImaging/Storage.c +++ b/libImaging/Storage.c @@ -373,7 +373,7 @@ ImagingAllocateArray(Imaging im, int dirty) { int y, line_in_block, current_block; ImagingMemoryArena arena = &ImagingDefaultArena; - char* p; + char* p = NULL; int linesize, lines_per_block, blocks_count; /* 0-width or 0-height image. No need to do anything */ From 44c2698f6967f233c24a53cb187b3e2036527eb3 Mon Sep 17 00:00:00 2001 From: Alexander Date: Mon, 18 Sep 2017 22:41:28 +0300 Subject: [PATCH 24/27] ImagingMemoryBlock structure --- Tests/test_core_resources.py | 2 ++ libImaging/Imaging.h | 9 ++++-- libImaging/Storage.c | 56 ++++++++++++++++++++---------------- 3 files changed, 41 insertions(+), 26 deletions(-) diff --git a/Tests/test_core_resources.py b/Tests/test_core_resources.py index 46ca12e386d..5fe632bed30 100644 --- a/Tests/test_core_resources.py +++ b/Tests/test_core_resources.py @@ -124,6 +124,7 @@ def test_set_blocks_max_stats(self): @unittest.skipIf(is_pypy, "images are not collected") def test_clear_cache_stats(self): Image.core.reset_stats() + Image.core.clear_cache() Image.core.set_blocks_max(128) Image.core.set_block_size(4096) Image.new('RGB', (256, 256)) @@ -131,6 +132,7 @@ def test_clear_cache_stats(self): Image.core.clear_cache() stats = Image.core.get_stats() + print(stats) self.assertGreaterEqual(stats['new_count'], 2) self.assertGreaterEqual(stats['allocated_blocks'], 64) self.assertGreaterEqual(stats['reused_blocks'], 64) diff --git a/libImaging/Imaging.h b/libImaging/Imaging.h index 14fac0225d0..556f3965e15 100644 --- a/libImaging/Imaging.h +++ b/libImaging/Imaging.h @@ -75,6 +75,11 @@ typedef struct ImagingPaletteInstance* ImagingPalette; #define IMAGING_MODE_LENGTH 6+1 /* Band names ("1", "L", "P", "RGB", "RGBA", "CMYK", "YCbCr", "BGR;xy") */ +typedef struct { + char *ptr; + int size; +} ImagingMemoryBlock; + struct ImagingMemoryInstance { /* Format */ @@ -95,7 +100,7 @@ struct ImagingMemoryInstance { /* Internals */ char **image; /* Actual raster data. */ char *block; /* Set if data is allocated in a single block. */ - char **blocks; /* Memory blocks for pixel storage */ + ImagingMemoryBlock *blocks; /* Memory blocks for pixel storage */ int pixelsize; /* Size of a pixel, in bytes (1, 2 or 4) */ int linesize; /* Size of a line, in bytes (xsize * pixelsize) */ @@ -158,7 +163,7 @@ typedef struct ImagingMemoryArena { int block_size; int blocks_max; int blocks_cached; - void **blocks; + ImagingMemoryBlock *blocks; int stats_new_count; int stats_allocated_blocks; int stats_reused_blocks; diff --git a/libImaging/Storage.c b/libImaging/Storage.c index 433e100cf95..7f12088a12c 100644 --- a/libImaging/Storage.c +++ b/libImaging/Storage.c @@ -284,14 +284,14 @@ ImagingMemorySetBlocksMax(ImagingMemoryArena arena, int blocks_max) free(arena->blocks); arena->blocks = NULL; } else if (arena->blocks != NULL) { - p = realloc(arena->blocks, sizeof(void*) * blocks_max); + p = realloc(arena->blocks, sizeof(*arena->blocks) * blocks_max); if ( ! p) { // Leave previous blocks_max value return 0; } arena->blocks = p; } else { - arena->blocks = calloc(sizeof(void*), blocks_max); + arena->blocks = calloc(sizeof(*arena->blocks), blocks_max); if ( ! arena->blocks) { return 0; } @@ -306,49 +306,57 @@ ImagingMemoryClearCache(ImagingMemoryArena arena, int new_size) { while (arena->blocks_cached > new_size) { arena->blocks_cached -= 1; - free(arena->blocks[arena->blocks_cached]); + free(arena->blocks[arena->blocks_cached].ptr); arena->stats_freed_blocks += 1; } } -void * +ImagingMemoryBlock memory_get_block(ImagingMemoryArena arena, int requested_size, int dirty) { - void *block; + ImagingMemoryBlock block = {NULL, 0}; + if (arena->blocks_cached > 0) { + // Get block from cache arena->blocks_cached -= 1; - block = realloc(arena->blocks[arena->blocks_cached], requested_size); - if ( ! block) { - free(arena->blocks[arena->blocks_cached]); + block = arena->blocks[arena->blocks_cached]; + // Reallocate if needed + if (block.size != requested_size){ + block.ptr = realloc(block.ptr, requested_size); + } + if ( ! block.ptr) { + // Can't allocate, free prevous pointer (it is still valid) + free(arena->blocks[arena->blocks_cached].ptr); arena->stats_freed_blocks += 1; - return NULL; + return block; + } + if ( ! dirty) { + memset(block.ptr, 0, requested_size); } arena->stats_reused_blocks += 1; - if (block != arena->blocks[arena->blocks_cached]) { + if (block.ptr != arena->blocks[arena->blocks_cached].ptr) { arena->stats_reallocated_blocks += 1; } - if ( ! dirty) { - memset(block, 0, requested_size); - } } else { if (dirty) { - block = malloc(requested_size); + block.ptr = malloc(requested_size); } else { - block = calloc(1, requested_size); + block.ptr = calloc(1, requested_size); } arena->stats_allocated_blocks += 1; } + block.size = requested_size; return block; } void -memory_return_block(ImagingMemoryArena arena, void *block) +memory_return_block(ImagingMemoryArena arena, ImagingMemoryBlock block) { if (arena->blocks_cached < arena->blocks_max) { arena->blocks[arena->blocks_cached] = block; arena->blocks_cached += 1; } else { - free(block); + free(block.ptr); arena->stats_freed_blocks += 1; } } @@ -360,7 +368,7 @@ ImagingDestroyArray(Imaging im) int y = 0; if (im->blocks) { - while (im->blocks[y]) { + while (im->blocks[y].ptr) { memory_return_block(&ImagingDefaultArena, im->blocks[y]); y += 1; } @@ -373,7 +381,7 @@ ImagingAllocateArray(Imaging im, int dirty) { int y, line_in_block, current_block; ImagingMemoryArena arena = &ImagingDefaultArena; - char* p = NULL; + ImagingMemoryBlock block = {NULL, 0}; int linesize, lines_per_block, blocks_count; /* 0-width or 0-height image. No need to do anything */ @@ -391,7 +399,7 @@ ImagingAllocateArray(Imaging im, int dirty) im->destroy = ImagingDestroyArray; /* One extra ponter is always NULL */ - im->blocks = (char **)calloc(sizeof(char *), blocks_count + 1); + im->blocks = calloc(sizeof(*im->blocks), blocks_count + 1); if ( ! im->blocks) { return (Imaging) ImagingError_MemoryError(); } @@ -406,14 +414,14 @@ ImagingAllocateArray(Imaging im, int dirty) if (lines_remained > im->ysize - y) { lines_remained = im->ysize - y; } - p = memory_get_block(arena, lines_remained * linesize, dirty); - if ( ! p) { + block = memory_get_block(arena, lines_remained * linesize, dirty); + if ( ! block.ptr) { return (Imaging) ImagingError_MemoryError(); } - im->blocks[current_block] = p; + im->blocks[current_block] = block; } - im->image[y] = p + linesize * line_in_block; + im->image[y] = block.ptr + linesize * line_in_block; line_in_block += 1; if (line_in_block >= lines_per_block) { From db08235c05eabcd5e0d98817afbbe0f09adbe6e1 Mon Sep 17 00:00:00 2001 From: Alexander Date: Tue, 19 Sep 2017 00:14:44 +0300 Subject: [PATCH 25/27] reduce size of blocks returned to pool --- _imaging.c | 2 +- libImaging/Storage.c | 5 +++++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/_imaging.c b/_imaging.c index ddbc303f87e..acba91bfd51 100644 --- a/_imaging.c +++ b/_imaging.c @@ -3473,7 +3473,7 @@ _set_blocks_max(PyObject* self, PyObject* args) static PyObject* _clear_cache(PyObject* self, PyObject* args) { - if (!PyArg_ParseTuple(args, ":_clear_cache")) + if (!PyArg_ParseTuple(args, ":clear_cache")) return NULL; ImagingMemoryClearCache(&ImagingDefaultArena, 0); diff --git a/libImaging/Storage.c b/libImaging/Storage.c index 7f12088a12c..e3084a07993 100644 --- a/libImaging/Storage.c +++ b/libImaging/Storage.c @@ -353,6 +353,11 @@ void memory_return_block(ImagingMemoryArena arena, ImagingMemoryBlock block) { if (arena->blocks_cached < arena->blocks_max) { + // Reduce block size + if (block.size > arena->block_size) { + block.size = arena->block_size; + block.ptr = realloc(block.ptr, arena->block_size); + } arena->blocks[arena->blocks_cached] = block; arena->blocks_cached += 1; } else { From 654b5f7958bc760bef06ed4a2b0beed004e5f1e8 Mon Sep 17 00:00:00 2001 From: Alexander Date: Tue, 19 Sep 2017 01:00:18 +0300 Subject: [PATCH 26/27] tests for env vars --- Tests/test_core_resources.py | 27 ++++++++++++++++++++++++++- 1 file changed, 26 insertions(+), 1 deletion(-) diff --git a/Tests/test_core_resources.py b/Tests/test_core_resources.py index 5fe632bed30..30bea3e802c 100644 --- a/Tests/test_core_resources.py +++ b/Tests/test_core_resources.py @@ -132,7 +132,6 @@ def test_clear_cache_stats(self): Image.core.clear_cache() stats = Image.core.get_stats() - print(stats) self.assertGreaterEqual(stats['new_count'], 2) self.assertGreaterEqual(stats['allocated_blocks'], 64) self.assertGreaterEqual(stats['reused_blocks'], 64) @@ -153,3 +152,29 @@ def test_large_images(self): self.assertEqual(stats['blocks_cached'], 0) if not is_pypy: self.assertGreaterEqual(stats['freed_blocks'], 16) + + +class TestEnvVars(PillowTestCase): + def tearDown(self): + # Restore default values + Image.core.set_alignment(1) + Image.core.set_block_size(1024*1024) + Image.core.set_blocks_max(0) + Image.core.clear_cache() + + def test_units(self): + Image._apply_env_variables({'PILLOW_BLOCKS_MAX': '2K'}) + self.assertEqual(Image.core.get_blocks_max(), 2*1024) + Image._apply_env_variables({'PILLOW_BLOCK_SIZE': '2m'}) + self.assertEqual(Image.core.get_block_size(), 2*1024*1024) + + def test_warnings(self): + self.assert_warning( + UserWarning, Image._apply_env_variables, + {'PILLOW_ALIGNMENT': '15'}) + self.assert_warning( + UserWarning, Image._apply_env_variables, + {'PILLOW_BLOCK_SIZE': '1024'}) + self.assert_warning( + UserWarning, Image._apply_env_variables, + {'PILLOW_BLOCKS_MAX': 'wat'}) From 5366a94725e5450e71772c19500d9d2cd4cf659d Mon Sep 17 00:00:00 2001 From: Alexander Date: Tue, 19 Sep 2017 03:10:57 +0300 Subject: [PATCH 27/27] clear cache at exit --- PIL/Image.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/PIL/Image.py b/PIL/Image.py index a10bb7eaea6..156925c040b 100644 --- a/PIL/Image.py +++ b/PIL/Image.py @@ -110,6 +110,7 @@ def __getattr__(self, id): import sys import io import struct +import atexit # type stuff import collections @@ -2861,3 +2862,4 @@ def _apply_env_variables(env=None): warnings.warn("{0}: {1}".format(var_name, e)) _apply_env_variables() +atexit.register(core.clear_cache)