SessionSerializer: Truly unique URIs

Before, only spent uris where sanitized not to be contained within
remaining uris. Change this so that each uri in the
union(remaining,spent) get saved once at most.
The order of the uris will won't be changed, with remaining uris going
first followed by spent uris.

Also avoid copying the uri std::strings around during dupe checking,
usually resulting in better performance regarding CPU and space.

src/SessionSerializer.cc

@@ -137,24 +137,41 @@ bool writeOption(IOFile& fp, const SharedHandle<Option>& op)
 } // namespace
 
 namespace {
-bool writeUri(IOFile& fp, const std::string& uri)
+  template<typename T>
-{
+  class Unique {
-  return fp.write(uri.c_str(), uri.size()) == uri.size() &&
+    typedef T type;
-    fp.write("\t", 1) == 1;
+    struct PointerCmp {
-}
+      inline bool operator()(const type* x, const type* y) {
-} // namespace
+        return *x < *y;
-
+      }
-namespace {
+    };
-template<typename InputIterator>
+    std::set<const type*, PointerCmp> known;
-bool writeUri(IOFile& fp, InputIterator first, InputIterator last)
+  public:
-{
+    inline bool operator()(const type& v) {
-  for(; first != last; ++first) {
+      return known.insert(&v).second;
-    if(!writeUri(fp, *first)) {
-      return false;
     }
+  };
+
+  bool writeUri(IOFile& fp, const std::string& uri)
+  {
+    return fp.write(uri.c_str(), uri.size()) == uri.size() &&
+      fp.write("\t", 1) == 1;
+  }
+
+  template<typename InputIterator, class UnaryPredicate>
+  bool writeUri(IOFile& fp, InputIterator first, InputIterator last,
+                UnaryPredicate& filter)
+  {
+    for(; first != last; ++first) {
+      if (!filter(*first)) {
+        continue;
+      }
+      if(!writeUri(fp, *first)) {
+        return false;
+      }
+    }
+    return true;
   }
-  return true;
-}
 } // namespace
 
 // The downloads whose followedBy() is empty is persisited with its
@@ -196,28 +213,26 @@ bool writeDownloadResult
     }
     const SharedHandle<FileEntry>& file = dr->fileEntries[0];
     // Don't save download if there are no URIs.
-    if(file->getRemainingUris().empty() &&
+    const bool hasRemaining = !file->getRemainingUris().empty();
-       file->getSpentUris().empty()) {
+    const bool hasSpent = !file->getSpentUris().empty();
+    if (!hasRemaining && !hasSpent) {
       return true;
     }
+
     // Save spent URIs + remaining URIs. Remove URI in spent URI which
     // also exists in remaining URIs.
-    std::set<std::string> uriSet(file->getRemainingUris().begin(),
+    {
-                                 file->getRemainingUris().end());
+      Unique<std::string> unique;
-    for(std::deque<std::string>::const_iterator i =
+      if (hasRemaining && !writeUri(fp, file->getRemainingUris().begin(),
-          file->getSpentUris().begin(), eoi = file->getSpentUris().end();
+                                    file->getRemainingUris().end(),
-        i != eoi; ++i) {
+                                    unique)) {
-      if(uriSet.count(*i)) {
+        return false;
-        continue;
+      }
-      }
+      if (hasSpent && !writeUri(fp, file->getSpentUris().begin(),
-      uriSet.insert(*i);
+                                file->getSpentUris().end(),
-      if(!writeUri(fp, *i)) {
+                                unique)) {
         return false;
       }
-    }
-    if(!writeUri(fp, file->getRemainingUris().begin(),
-                 file->getRemainingUris().end())) {
-      return false;
     }
     if(fp.write("\n", 1) != 1) {
       return false;

test/SessionSerializerTest.cc

@@ -61,8 +61,19 @@ void SessionSerializerTest::testSave()
   };
   // This URI will be discarded because same URI exists in remaining
   // URIs.
+  drs[1]->fileEntries[0]->getRemainingUris().push_back("http://error");
+  drs[1]->fileEntries[0]->getRemainingUris().push_back("http://error3");
+  // This URI will be discarded because same URI exists in remaining
+  // URIs.
+  drs[1]->fileEntries[0]->getRemainingUris().push_back("http://error");
+  //
+  // This URI will be discarded because same URI exists in remaining
+  // URIs.
   drs[1]->fileEntries[0]->getSpentUris().push_back("http://error");
   drs[1]->fileEntries[0]->getSpentUris().push_back("http://error2");
+  // This URI will be discarded because same URI exists in remaining
+  // URIs.
+  drs[1]->fileEntries[0]->getSpentUris().push_back("http://error");
 
   drs[3]->option->put(PREF_FORCE_SAVE, A2_V_TRUE);
   for(size_t i = 0; i < sizeof(drs)/sizeof(drs[0]); ++i) {
@@ -79,7 +90,7 @@ void SessionSerializerTest::testSave()
   std::ifstream ss(filename.c_str(), std::ios::binary);
   std::string line;
   std::getline(ss, line);
-  CPPUNIT_ASSERT_EQUAL(std::string("http://error2\thttp://error\t"), line);
+  CPPUNIT_ASSERT_EQUAL(std::string("http://error\thttp://error3\thttp://error2\t"), line);
   std::getline(ss, line);
   CPPUNIT_ASSERT_EQUAL(fmt(" gid=%s", drs[1]->gid->toHex().c_str()), line);
   std::getline(ss, line);