Fix excessive usage of malloc/free

Split ZopfliInitHash to: - ZopfliAllocHash - allocate hash memory, - ZopfliResetHash - reset hash values. Allocate Hash outside of ZopfliLZ77Greedy and ZopfliLZ77OptimalRun that pass it further to functions previously allocating them. Do the same for costs malloc'd array. Reason for this change: - the size of malloc doesn't change, - speed up Zopfli*, - fix crash on certain devices**. * speeds up Zopfli (especially on smaller blocks) by reducing amount of sys time from ~7s to 0.1s on x64 Linux for ~5m compression time and from ~1m to 0.1s on ARMv7 Linux for 13m compression time. ** fixes a large amount of iterations crash on some ARM devices that due to architecture or older kernel (not sure which) don't handle too aggressive heap allocation and freeing.
google · Apr 6, 2016 · 365bda1 · 365bda1
1 parent 16e0741
commit 365bda1
Show file tree

Hide file tree

Showing 6 changed files with 59 additions and 45 deletions.
diff --git a/src/zopfli/blocksplitter.c b/src/zopfli/blocksplitter.c
@@ -281,16 +281,19 @@ void ZopfliBlockSplit(const ZopfliOptions* options,
   size_t* lz77splitpoints = 0;
   size_t nlz77points = 0;
   ZopfliLZ77Store store;
+  ZopfliHash hash;
+  ZopfliHash* h = &hash;
 
   ZopfliInitLZ77Store(in, &store);
   ZopfliInitBlockState(options, instart, inend, 0, &s);
+  ZopfliAllocHash(ZOPFLI_WINDOW_SIZE, h);
 
   *npoints = 0;
   *splitpoints = 0;
 
   /* Unintuitively, Using a simple LZ77 method here instead of ZopfliLZ77Optimal
   results in better blocks. */
-  ZopfliLZ77Greedy(&s, in, instart, inend, &store);
+  ZopfliLZ77Greedy(&s, in, instart, inend, &store, h);
 
   ZopfliBlockSplitLZ77(options,
                        &store, maxblocks,
@@ -313,6 +316,7 @@ void ZopfliBlockSplit(const ZopfliOptions* options,
   free(lz77splitpoints);
   ZopfliCleanBlockState(&s);
   ZopfliCleanLZ77Store(&store);
+  ZopfliCleanHash(h);
 }
 
 void ZopfliBlockSplitSimple(const unsigned char* in,

diff --git a/src/zopfli/hash.c b/src/zopfli/hash.c
@@ -26,13 +26,26 @@ Author: [email protected] (Jyrki Alakuijala)
 #define HASH_SHIFT 5
 #define HASH_MASK 32767
 
-void ZopfliInitHash(size_t window_size, ZopfliHash* h) {
-  size_t i;
-
-  h->val = 0;
+void ZopfliAllocHash(size_t window_size, ZopfliHash* h) {
   h->head = (int*)malloc(sizeof(*h->head) * 65536);
   h->prev = (unsigned short*)malloc(sizeof(*h->prev) * window_size);
   h->hashval = (int*)malloc(sizeof(*h->hashval) * window_size);
+
+#ifdef ZOPFLI_HASH_SAME
+  h->same = (unsigned short*)malloc(sizeof(*h->same) * window_size);
+#endif
+
+#ifdef ZOPFLI_HASH_SAME_HASH
+  h->head2 = (int*)malloc(sizeof(*h->head2) * 65536);
+  h->prev2 = (unsigned short*)malloc(sizeof(*h->prev2) * window_size);
+  h->hashval2 = (int*)malloc(sizeof(*h->hashval2) * window_size);
+#endif
+}
+
+void ZopfliResetHash(size_t window_size, ZopfliHash* h) {
+  size_t i;
+
+  h->val = 0;
   for (i = 0; i < 65536; i++) {
     h->head[i] = -1;  /* -1 indicates no head so far. */
   }
@@ -42,17 +55,13 @@ void ZopfliInitHash(size_t window_size, ZopfliHash* h) {
   }
 
 #ifdef ZOPFLI_HASH_SAME
-  h->same = (unsigned short*)malloc(sizeof(*h->same) * window_size);
   for (i = 0; i < window_size; i++) {
     h->same[i] = 0;
   }
 #endif
 
 #ifdef ZOPFLI_HASH_SAME_HASH
   h->val2 = 0;
-  h->head2 = (int*)malloc(sizeof(*h->head2) * 65536);
-  h->prev2 = (unsigned short*)malloc(sizeof(*h->prev2) * window_size);
-  h->hashval2 = (int*)malloc(sizeof(*h->hashval2) * window_size);
   for (i = 0; i < 65536; i++) {
     h->head2[i] = -1;
   }

diff --git a/src/zopfli/hash.h b/src/zopfli/hash.h
@@ -46,10 +46,13 @@ typedef struct ZopfliHash {
 #endif
 } ZopfliHash;
 
-/* Allocates and initializes all fields of ZopfliHash. */
-void ZopfliInitHash(size_t window_size, ZopfliHash* h);
+/* Allocates ZopfliHash memory. */
+void ZopfliAllocHash(size_t window_size, ZopfliHash* h);
 
-/* Frees all fields of ZopfliHash. */
+/* Resets all fields of ZopfliHash. */
+void ZopfliResetHash(size_t window_size, ZopfliHash* h);
+
+/* Frees ZopfliHash memory. */
 void ZopfliCleanHash(ZopfliHash* h);
 
 /*

diff --git a/src/zopfli/lz77.c b/src/zopfli/lz77.c
@@ -542,7 +542,7 @@ void ZopfliFindLongestMatch(ZopfliBlockState* s, const ZopfliHash* h,
 
 void ZopfliLZ77Greedy(ZopfliBlockState* s, const unsigned char* in,
                       size_t instart, size_t inend,
-                      ZopfliLZ77Store* store) {
+                      ZopfliLZ77Store* store, ZopfliHash* h) {
   size_t i = 0, j;
   unsigned short leng;
   unsigned short dist;
@@ -551,9 +551,6 @@ void ZopfliLZ77Greedy(ZopfliBlockState* s, const unsigned char* in,
       ? instart - ZOPFLI_WINDOW_SIZE : 0;
   unsigned short dummysublen[259];
 
-  ZopfliHash hash;
-  ZopfliHash* h = &hash;
-
 #ifdef ZOPFLI_LAZY_MATCHING
   /* Lazy matching. */
   unsigned prev_length = 0;
@@ -564,7 +561,7 @@ void ZopfliLZ77Greedy(ZopfliBlockState* s, const unsigned char* in,
 
   if (instart == inend) return;
 
-  ZopfliInitHash(ZOPFLI_WINDOW_SIZE, h);
+  ZopfliResetHash(ZOPFLI_WINDOW_SIZE, h);
   ZopfliWarmupHash(in, windowstart, inend, h);
   for (i = windowstart; i < instart; i++) {
     ZopfliUpdateHash(in, i, inend, h);
@@ -629,6 +626,4 @@ void ZopfliLZ77Greedy(ZopfliBlockState* s, const unsigned char* in,
       ZopfliUpdateHash(in, i, inend, h);
     }
   }
-
-  ZopfliCleanHash(h);
 }
diff --git a/src/zopfli/lz77.h b/src/zopfli/lz77.h
@@ -137,6 +137,6 @@ dictionary.
 */
 void ZopfliLZ77Greedy(ZopfliBlockState* s, const unsigned char* in,
                       size_t instart, size_t inend,
-                      ZopfliLZ77Store* store);
+                      ZopfliLZ77Store* store, ZopfliHash* h);
 
 #endif  /* ZOPFLI_LZ77_H_ */
diff --git a/src/zopfli/squeeze.c b/src/zopfli/squeeze.c
@@ -213,27 +213,22 @@ static double GetBestLengths(ZopfliBlockState *s,
                              const unsigned char* in,
                              size_t instart, size_t inend,
                              CostModelFun* costmodel, void* costcontext,
-                             unsigned short* length_array) {
+                             unsigned short* length_array,
+                             ZopfliHash* h, float* costs) {
   /* Best cost to get here so far. */
   size_t blocksize = inend - instart;
-  float* costs;
   size_t i = 0, k;
   unsigned short leng;
   unsigned short dist;
   unsigned short sublen[259];
   size_t windowstart = instart > ZOPFLI_WINDOW_SIZE
       ? instart - ZOPFLI_WINDOW_SIZE : 0;
-  ZopfliHash hash;
-  ZopfliHash* h = &hash;
   double result;
   double mincost = GetCostModelMinCost(costmodel, costcontext);
 
   if (instart == inend) return 0;
 
-  costs = (float*)malloc(sizeof(float) * (blocksize + 1));
-  if (!costs) exit(-1); /* Allocation failed. */
-
-  ZopfliInitHash(ZOPFLI_WINDOW_SIZE, h);
+  ZopfliResetHash(ZOPFLI_WINDOW_SIZE, h);
   ZopfliWarmupHash(in, windowstart, inend, h);
   for (i = windowstart; i < instart; i++) {
     ZopfliUpdateHash(in, i, inend, h);
@@ -302,9 +297,6 @@ static double GetBestLengths(ZopfliBlockState *s,
   assert(costs[blocksize] >= 0);
   result = costs[blocksize];
 
-  ZopfliCleanHash(h);
-  free(costs);
-
   return result;
 }
 
@@ -338,19 +330,16 @@ static void TraceBackwards(size_t size, const unsigned short* length_array,
 static void FollowPath(ZopfliBlockState* s,
                        const unsigned char* in, size_t instart, size_t inend,
                        unsigned short* path, size_t pathsize,
-                       ZopfliLZ77Store* store) {
+                       ZopfliLZ77Store* store, ZopfliHash *h) {
   size_t i, j, pos = 0;
   size_t windowstart = instart > ZOPFLI_WINDOW_SIZE
       ? instart - ZOPFLI_WINDOW_SIZE : 0;
 
   size_t total_length_test = 0;
 
-  ZopfliHash hash;
-  ZopfliHash* h = &hash;
-
   if (instart == inend) return;
 
-  ZopfliInitHash(ZOPFLI_WINDOW_SIZE, h);
+  ZopfliResetHash(ZOPFLI_WINDOW_SIZE, h);
   ZopfliWarmupHash(in, windowstart, inend, h);
   for (i = windowstart; i < instart; i++) {
     ZopfliUpdateHash(in, i, inend, h);
@@ -389,8 +378,6 @@ static void FollowPath(ZopfliBlockState* s,
 
     pos += length;
   }
-
-  ZopfliCleanHash(h);
 }
 
 /* Calculates the entropy of the statistics */
@@ -436,14 +423,15 @@ static double LZ77OptimalRun(ZopfliBlockState* s,
     const unsigned char* in, size_t instart, size_t inend,
     unsigned short** path, size_t* pathsize,
     unsigned short* length_array, CostModelFun* costmodel,
-    void* costcontext, ZopfliLZ77Store* store) {
-  double cost = GetBestLengths(
-      s, in, instart, inend, costmodel, costcontext, length_array);
+    void* costcontext, ZopfliLZ77Store* store,
+    ZopfliHash* h, float* costs) {
+  double cost = GetBestLengths(s, in, instart, inend, costmodel,
+                costcontext, length_array, h, costs);
   free(*path);
   *path = 0;
   *pathsize = 0;
   TraceBackwards(inend - instart, length_array, path, pathsize);
-  FollowPath(s, in, instart, inend, *path, *pathsize, store);
+  FollowPath(s, in, instart, inend, *path, *pathsize, store, h);
   assert(cost < ZOPFLI_LARGE_FLOAT);
   return cost;
 }
@@ -459,26 +447,31 @@ void ZopfliLZ77Optimal(ZopfliBlockState *s,
   unsigned short* path = 0;
   size_t pathsize = 0;
   ZopfliLZ77Store currentstore;
+  ZopfliHash hash;
+  ZopfliHash* h = &hash;
   SymbolStats stats, beststats, laststats;
   int i;
+  float* costs = (float*)malloc(sizeof(float) * (blocksize + 1));
   double cost;
   double bestcost = ZOPFLI_LARGE_FLOAT;
   double lastcost = 0;
   /* Try randomizing the costs a bit once the size stabilizes. */
   RanState ran_state;
   int lastrandomstep = -1;
 
+  if (!costs) exit(-1); /* Allocation failed. */
   if (!length_array) exit(-1); /* Allocation failed. */
 
   InitRanState(&ran_state);
   InitStats(&stats);
   ZopfliInitLZ77Store(in, &currentstore);
+  ZopfliAllocHash(ZOPFLI_WINDOW_SIZE, h);
 
   /* Do regular deflate, then loop multiple shortest path runs, each time using
   the statistics of the previous run. */
 
   /* Initial run. */
-  ZopfliLZ77Greedy(s, in, instart, inend, &currentstore);
+  ZopfliLZ77Greedy(s, in, instart, inend, &currentstore, h);
   GetStatistics(&currentstore, &stats);
 
   /* Repeat statistics with each time the cost model from the previous stat
@@ -488,7 +481,7 @@ void ZopfliLZ77Optimal(ZopfliBlockState *s,
     ZopfliInitLZ77Store(in, &currentstore);
     LZ77OptimalRun(s, in, instart, inend, &path, &pathsize,
                    length_array, GetCostStat, (void*)&stats,
-                   &currentstore);
+                   &currentstore, h, costs);
     cost = ZopfliCalculateBlockSize(&currentstore, 0, currentstore.size, 2);
     if (s->options->verbose_more || (s->options->verbose && cost < bestcost)) {
       fprintf(stderr, "Iteration %d: %d bit\n", i, (int) cost);
@@ -520,7 +513,9 @@ void ZopfliLZ77Optimal(ZopfliBlockState *s,
 
   free(length_array);
   free(path);
+  free(costs);
   ZopfliCleanLZ77Store(&currentstore);
+  ZopfliCleanHash(h);
 }
 
 void ZopfliLZ77OptimalFixed(ZopfliBlockState *s,
@@ -534,17 +529,25 @@ void ZopfliLZ77OptimalFixed(ZopfliBlockState *s,
       (unsigned short*)malloc(sizeof(unsigned short) * (blocksize + 1));
   unsigned short* path = 0;
   size_t pathsize = 0;
+  ZopfliHash hash;
+  ZopfliHash* h = &hash;
+  float* costs = (float*)malloc(sizeof(float) * (blocksize + 1));
 
+  if (!costs) exit(-1); /* Allocation failed. */
   if (!length_array) exit(-1); /* Allocation failed. */
 
+  ZopfliAllocHash(ZOPFLI_WINDOW_SIZE, h);
+
   s->blockstart = instart;
   s->blockend = inend;
 
   /* Shortest path for fixed tree This one should give the shortest possible
   result for fixed tree, no repeated runs are needed since the tree is known. */
   LZ77OptimalRun(s, in, instart, inend, &path, &pathsize,
-                 length_array, GetCostFixed, 0, store);
+                 length_array, GetCostFixed, 0, store, h, costs);
 
   free(length_array);
   free(path);
+  free(costs);
+  ZopfliCleanHash(h);
 }