Making it more standard

Author: lemire

CMakeLists.txt

@@ -114,6 +114,7 @@ add_library(FastPFOR STATIC
     src/bitpackingunaligned.cpp
     src/horizontalbitpacking.cpp
     src/simdunalignedbitpacking.cpp
+    src/codecfactory.cpp
     src/simdbitpacking.cpp
     src/varintdecode.c
     src/streamvbyte.c)

example.cpp

@@ -14,9 +14,10 @@
 
 int main() {
   using namespace FastPForLib;
+  CODECFactory factory;
 
   // We pick a CODEC
-  IntegerCODEC &codec = *CODECFactory::getFromName("simdfastpfor256");
+  IntegerCODEC &codec = *factory.getFromName("simdfastpfor256");
   // could use others, e.g., "simdbinarypacking", "varintg8iu"
   ////////////
   //

headers/codecfactory.h

@@ -41,45 +41,19 @@ namespace FastPForLib {
 typedef std::map<std::string, std::shared_ptr<IntegerCODEC>> CodecMap;
 
 /**
- * This class is a convenience class to generate codecs quickly.
- * It cannot be used safely in a multithreaded context where
- * each thread should have a different IntegerCODEC.
+ * You should have at least one factory per thread.
  */
 class CODECFactory {
 public:
-  static CodecMap scodecmap;
+  CODECFactory();
 
-  // hacked for convenience
-  static std::vector<std::shared_ptr<IntegerCODEC>> allSchemes() {
-    std::vector<std::shared_ptr<IntegerCODEC>> ans;
-    for (auto i = scodecmap.begin(); i != scodecmap.end(); ++i) {
-      ans.push_back(i->second);
-    }
-    return ans;
-  }
+  std::vector<std::shared_ptr<IntegerCODEC>> allSchemes();
 
-  static std::vector<std::string> allNames() {
-    std::vector<std::string> ans;
-    for (auto i = scodecmap.begin(); i != scodecmap.end(); ++i) {
-      ans.push_back(i->first);
-    }
-    return ans;
-  }
+  std::vector<std::string> allNames();
 
-  static std::shared_ptr<IntegerCODEC> &getFromName(std::string name) {
-    if (scodecmap.find(name) == scodecmap.end()) {
-      std::cerr << "name " << name << " does not refer to a CODEC."
-                << std::endl;
-      std::cerr << "possible choices:" << std::endl;
-      for (auto i = scodecmap.begin(); i != scodecmap.end(); ++i) {
-        std::cerr << static_cast<std::string>(i->first)
-                  << std::endl; // useless cast, but just to be clear
-      }
-      std::cerr << "for now, I'm going to just return 'copy'" << std::endl;
-      return scodecmap["copy"];
-    }
-    return scodecmap[name];
-  }
+  std::shared_ptr<IntegerCODEC> &getFromName(std::string name);
+private:
+  CodecMap scodecmap;
 };
 
 

src/codecfactory.cpp

@@ -1,12 +1,35 @@
 #include "codecfactory.h"
 
-/**
- * We moved part of the factor to its own cpp file because some users
- * create multiple instances. Note that the factory is not meant to be
- * a safe class, it is a convenience class and you should just have one
- * instance of it per project.
- */
 namespace FastPForLib {
+std::vector<std::shared_ptr<IntegerCODEC>> CODECFactory::allSchemes() {
+  std::vector<std::shared_ptr<IntegerCODEC>> ans;
+  for (auto i = scodecmap.begin(); i != scodecmap.end(); ++i) {
+    ans.push_back(i->second);
+  }
+  return ans;
+}
+
+std::vector<std::string> CODECFactory::allNames() {
+  std::vector<std::string> ans;
+  for (auto i = scodecmap.begin(); i != scodecmap.end(); ++i) {
+    ans.push_back(i->first);
+  }
+  return ans;
+}
+
+std::shared_ptr<IntegerCODEC> &CODECFactory::getFromName(std::string name) {
+  if (scodecmap.find(name) == scodecmap.end()) {
+    std::cerr << "name " << name << " does not refer to a CODEC." << std::endl;
+    std::cerr << "possible choices:" << std::endl;
+    for (auto i = scodecmap.begin(); i != scodecmap.end(); ++i) {
+      std::cerr << static_cast<std::string>(i->first)
+                << std::endl; // useless cast, but just to be clear
+    }
+    std::cerr << "for now, I'm going to just return 'copy'" << std::endl;
+    return scodecmap["copy"];
+  }
+  return scodecmap[name];
+}
 
 // C++11 allows better than this, but neither Microsoft nor Intel support C++11
 // fully.
@@ -41,13 +64,13 @@ inline CodecMap initializefactory() {
   map["pfor2008"] = std::shared_ptr<IntegerCODEC>(
       new CompositeCodec<PFor2008, VariableByte>());
   map["simdnewpfor"] = std::shared_ptr<IntegerCODEC>(
-      new CompositeCodec<SIMDNewPFor<4, Simple16<false> >, VariableByte>());
+      new CompositeCodec<SIMDNewPFor<4, Simple16<false>>, VariableByte>());
   map["newpfor"] = std::shared_ptr<IntegerCODEC>(
-      new CompositeCodec<NewPFor<4, Simple16<false> >, VariableByte>());
+      new CompositeCodec<NewPFor<4, Simple16<false>>, VariableByte>());
   map["optpfor"] = std::shared_ptr<IntegerCODEC>(
-      new CompositeCodec<OPTPFor<4, Simple16<false> >, VariableByte>());
+      new CompositeCodec<OPTPFor<4, Simple16<false>>, VariableByte>());
   map["simdoptpfor"] = std::shared_ptr<IntegerCODEC>(
-      new CompositeCodec<SIMDOPTPFor<4, Simple16<false> >, VariableByte>());
+      new CompositeCodec<SIMDOPTPFor<4, Simple16<false>>, VariableByte>());
   map["varint"] = std::shared_ptr<IntegerCODEC>(new VariableByte());
   map["vbyte"] = std::shared_ptr<IntegerCODEC>(new VByte());
   map["maskedvbyte"] = std::shared_ptr<IntegerCODEC>(new MaskedVByte());
@@ -73,5 +96,5 @@ inline CodecMap initializefactory() {
   map["copy"] = std::shared_ptr<IntegerCODEC>(new JustCopy());
   return map;
 }
-CodecMap CODECFactory::scodecmap = initializefactory();
-}
+CODECFactory::CODECFactory() : scodecmap(initializefactory())  {}
+} // namespace FastPForLib

src/codecs.cpp

@@ -727,7 +727,8 @@ void message() {
   cout << "the --codecs flag takes as an argument"
           " a comma-separated list of schemes chosen among those:"
        << endl;
-  vector<string> all = CODECFactory::allNames();
+  CODECFactory factory;
+  vector<string> all = factory.allNames();
   for (auto i = all.begin(); i != all.end(); ++i) {
     cout << *i;
     if (i + 1 == all.end())
@@ -751,8 +752,9 @@ int main(int argc, char **argv) {
   bool computeentropy = false;
 
   bool splitlongarrays = true;
+  CODECFactory factory;
   vector<shared_ptr<IntegerCODEC>> tmp =
-      CODECFactory::allSchemes(); // the default
+      factory.allSchemes(); // the default
   vector<algostats> myalgos;
   for (auto &i : tmp) {
     myalgos.push_back(algostats(i));
@@ -780,9 +782,9 @@ int main(int argc, char **argv) {
           cout << "# pretty name = " << *i << endl;
           if (i->at(0) == '@') { // SIMD
             myalgos.push_back(algostats(
-                CODECFactory::getFromName(i->substr(1, i->size() - 1)), true));
+                factory.getFromName(i->substr(1, i->size() - 1)), true));
           } else {
-            myalgos.push_back(algostats(CODECFactory::getFromName(*i)));
+            myalgos.push_back(algostats(factory.getFromName(*i)));
           }
           cout << "# added '" << myalgos.back().name() << "'" << endl;
         }

src/inmemorybenchmark.cpp

@@ -697,7 +697,8 @@ void message(const char *prog) {
        << endl;
   cerr << "Use the --codecs flag to specify the schemes." << endl;
   cerr << " schemes include:" << endl;
-  vector<string> all = CODECFactory::allNames();
+  CODECFactory factory;
+  vector<string> all = factory.allNames();
   for (auto i = all.begin(); i != all.end(); ++i) {
     cerr << *i << endl;
   }
@@ -713,8 +714,9 @@ int main(int argc, char **argv) {
   size_t MINLENGTH = 1;
   size_t MAXLENGTH = (std::numeric_limits<uint32_t>::max)();
   size_t MAXCOUNTER = (std::numeric_limits<std::size_t>::max)();
+  CODECFactory factory;
   vector<shared_ptr<IntegerCODEC>> tmp =
-      CODECFactory::allSchemes(); // the default
+      factory.allSchemes(); // the default
   vector<algostats> myalgos;
   for (auto i = tmp.begin(); i != tmp.end(); ++i) {
     myalgos.push_back(algostats(*i));
@@ -759,9 +761,9 @@ int main(int argc, char **argv) {
           if (i->at(0) == '@') { // SIMD
             string namewithoutprefix = i->substr(1, i->size() - 1);
             myalgos.push_back(
-                algostats(CODECFactory::getFromName(namewithoutprefix), true));
+                algostats(factory.getFromName(namewithoutprefix), true));
           } else {
-            myalgos.push_back(algostats(CODECFactory::getFromName(*i)));
+            myalgos.push_back(algostats(factory.getFromName(*i)));
           }
           cout << "# added '" << myalgos.back().name() << "'" << endl;
         }

src/unit.cpp

@@ -18,7 +18,8 @@ using namespace std;
 using namespace FastPForLib;
 
 int main() {
-  vector<shared_ptr<IntegerCODEC>> myalgos = CODECFactory::allSchemes();
+  CODECFactory factory;
+  vector<shared_ptr<IntegerCODEC>> myalgos = factory.allSchemes();
   for (uint32_t b = 0; b <= 28; ++b) {
 
     cout << "testing... b = " << b << endl;