[poppler] Poppler too slow when compiled with cms

[Koji Otani]

I made a patch for this problem and attached it.
Please try and check it.

(1) It caches last 5 GfxICCBasedColorSpace.
(2) It shares GfxColorTransform objects not GfxICCBasedColorSpace, and
the cache holds copies of GfxICCBasedColorSpace.
(3) GfxColorTransform objects are reference counted before this patch.
(4) To check if two ICCBasedColorSpace are same, it uses number and
    generation of ICCProfile Stream (PDF) Objects. 
    Please note that stream object must be indirect object.
------------
Koji Otani

From: Albert Astals Cid <aacid at kde.org>
Subject: Re: [poppler] Poppler too slow when compiled with cms
Date: Mon, 19 Jan 2009 20:00:39 +0100
Message-ID: <200901192000.40478.aacid at kde.org>

aacid> A Dilluns 19 Gener 2009, Koji Otani va escriure:
aacid> > From: Albert Astals Cid <aacid at kde.org>
aacid> > Subject: Re: [poppler] Poppler too slow when compiled with cms
aacid> > Date: Sun, 18 Jan 2009 20:25:03 +0100
aacid> > Message-ID: <200901182025.03909.aacid at kde.org>
aacid> >
aacid> > aacid> A Diumenge 18 Gener 2009, Carlos Garcia Campos va escriure:
aacid> > aacid> > when compiled with cms support enabled poppler is too slow when
aacid> > aacid> > rendering some pages. For example, rendering page 831 of PDF
aacid> > aacid> > Specification 1.7 takes more than 1 minute in my system.
aacid> > aacid> >
aacid> > aacid> > Koji, any idea why does it happen?
aacid> > aacid>
aacid> > aacid> I think the problem is that in this page there's a Pattern, our
aacid> > slowest code, aacid> that forces the creation of 272 GfxICCBasedColorSpaces
aacid> > that isn't fast aacid> either, but the problem is not the
aacid> > GfxICCBasedColorSpaces itself, the problem aacid> is that the pattern
aacid> > implementation sucks.
aacid> > aacid>
aacid> > aacid> To try to bypass this problem we could try implement a kind of cache
aacid> > for aacid> GfxColorSpace so you don't have to recreate one if is the same
aacid> > as one of the aacid> last N (1-5). This has several implications, like
aacid> > adding refcounting to aacid> GfxColorSpace and finding how to know if we
aacid> > are asked for the same aacid> GfxColorSpace.
aacid> > aacid>
aacid> >
aacid> > I think you are right.
aacid> > I did quick hack to test only this case. It caches
aacid> >  a last GfxICCBasedColorSpace.
aacid> >
aacid> >   It created only 3 GfxICCBasedColorSpaces for rendering the page.
aacid> >   It took 2 seconds. (before hack, it took 140 seconds)
aacid> 
aacid> Can we see the patch? Maybe we can improve it a bit or use it directly.
aacid> 
aacid> Albert
aacid> 
aacid> >
aacid> > -----------------
aacid> > Koji Otani
aacid> >
aacid> _______________________________________________
aacid> poppler mailing list
aacid> poppler at lists.freedesktop.org
aacid> http://lists.freedesktop.org/mailman/listinfo/poppler
-------------- next part --------------
diff --git a/poppler/GfxState.cc b/poppler/GfxState.cc
index 913245b..0de4840 100644
--- a/poppler/GfxState.cc
+++ b/poppler/GfxState.cc
@@ -1410,6 +1410,12 @@ GfxColorSpace *GfxICCBasedColorSpace::parse(Array *arr) {
     iccProfileStreamA.gen = 0;
   }
   obj1.free();
+#ifdef USE_CMS
+  // check cache
+  if (iccProfileStreamA.num > 0
+     && (cs = GfxICCBasedCache::lookup(iccProfileStreamA.num,
+          iccProfileStreamA.gen)) != NULL) return cs;
+#endif
   arr->get(1, &obj1);
   if (!obj1.isStream()) {
     error(-1, "Bad ICCBased color space (stream)");
@@ -1516,6 +1522,10 @@ GfxColorSpace *GfxICCBasedColorSpace::parse(Array *arr) {
     cmsCloseProfile(hp);
   }
   obj1.free();
+  // put this colorSpace into cache
+  if (iccProfileStreamA.num > 0) {
+    GfxICCBasedCache::put(iccProfileStreamA.num,iccProfileStreamA.gen,cs);
+  }
 #endif
   return cs;
 }
@@ -1654,6 +1664,56 @@ void GfxICCBasedColorSpace::getDefaultRanges(double *decodeLow,
 #endif
 }
 
+#ifdef USE_CMS
+GfxICCBasedCache
+   GfxICCBasedCache::cache[GFX_ICCBASED_CACHE_SIZE];
+
+GfxICCBasedCache::GfxICCBasedCache()
+{
+  num = 0;
+  gen = 0;
+  colorSpace = 0;
+}
+
+GfxICCBasedColorSpace *GfxICCBasedCache::lookup(int numA, int genA)
+{
+  int i;
+
+  if (cache[0].num == numA && cache[0].gen == genA) {
+    return (GfxICCBasedColorSpace *)cache[0].colorSpace->copy();
+  }
+  for (i = 1;i < GFX_ICCBASED_CACHE_SIZE && cache[i].num > 0;i++) {
+    if (cache[i].num == numA && cache[i].gen == genA) {
+      int j;
+      GfxICCBasedCache hit = cache[i];
+
+      for (j = 0;j < i;j++) {
+	cache[j+1] = cache[j];
+      }
+      cache[0] = hit;
+      return (GfxICCBasedColorSpace *)hit.colorSpace->copy();
+    }
+  }
+  return NULL;
+}
+
+void GfxICCBasedCache::put(int numA, int genA,
+  GfxICCBasedColorSpace *cs)
+{
+  int i;
+
+  if (cache[GFX_ICCBASED_CACHE_SIZE-1].num > 0) {
+    delete cache[GFX_ICCBASED_CACHE_SIZE-1].colorSpace;
+  }
+  for (i = 0;i < GFX_ICCBASED_CACHE_SIZE-1 && cache[i].num > 0;i++) {
+    cache[i+1] = cache[i];
+  }
+  cache[0].num = numA;
+  cache[0].gen = genA;
+  cache[0].colorSpace = (GfxICCBasedColorSpace *)cs->copy();
+}
+#endif
+
 //------------------------------------------------------------------------
 // GfxIndexedColorSpace
 //------------------------------------------------------------------------
diff --git a/poppler/GfxState.h b/poppler/GfxState.h
index 1a0aacf..ad5fae6 100644
--- a/poppler/GfxState.h
+++ b/poppler/GfxState.h
@@ -497,6 +497,21 @@ private:
 #endif
 };
 
+#ifdef USE_CMS
+#define GFX_ICCBASED_CACHE_SIZE 5
+class GfxICCBasedCache {
+public:
+  static GfxICCBasedColorSpace *lookup(int numA, int genA);
+  static void put(int numA, int genA, GfxICCBasedColorSpace *cs);
+private:
+  GfxICCBasedCache();
+  int num;
+  int gen;
+  GfxICCBasedColorSpace *colorSpace;
+  static GfxICCBasedCache cache[GFX_ICCBASED_CACHE_SIZE];
+};
+#endif
+
 //------------------------------------------------------------------------
 // GfxIndexedColorSpace
 //------------------------------------------------------------------------