[lmi-commits] [lmi] odd/string_db5 659dabd 2/3: Cache class product_data too

[Greg Chicares]

branch: odd/string_db5
commit 659dabd160ae3bf2695db57b103c9a76d5bdc2f0
Author: Gregory W. Chicares <gchicares@sbcglobal.net>
Commit: Gregory W. Chicares <gchicares@sbcglobal.net>

    Cache class product_data too
    
    The product_data ctor taking a string argument expects a product name
    like 'sample2xyz', whereas the corresponding filename is something like
      /opt/lmi/data/sample2.xyz
    and mixin class cache_file_reads requires a filename. Explored the idea
    of adding
      product_data::product_data(ce_product_name)
    which is the proper abstraction for a product name, but hesitated to
    make product_data (which is of central importance) dependent on
    ce_product_name (which depends on the runtime environment). Decided to
    add
      product_data::product_data(filesystem::path)
    which is the proper abstraction for a filepath, and to use that
    abstraction instead of std::string in class cache_file_reads.
    
    This change yielded some improvement in 'make cli_timing', but less than
    hoped--presumably because product_data objects created elsewhere are not
    yet cached.
---
 Speed_gcc_i686-w64-mingw32    | 12 ++++++------
 Speed_gcc_x86_64-pc-linux-gnu | 12 ++++++------
 cache_file_reads.hpp          | 10 ++++++----
 cache_file_reads_test.cpp     | 11 +++++++----
 dbdict.cpp                    |  4 ++--
 dbdict.hpp                    |  4 +++-
 ihs_basicval.cpp              |  9 +++++++--
 lingo.cpp                     |  4 ++--
 lingo.hpp                     |  4 +++-
 product_data.cpp              | 38 ++++++++++++++++++++++++++++++++------
 product_data.hpp              | 10 ++++++++++
 11 files changed, 84 insertions(+), 34 deletions(-)

diff --git a/Speed_gcc_i686-w64-mingw32 b/Speed_gcc_i686-w64-mingw32
index b34a40b..135f421 100644
--- a/Speed_gcc_i686-w64-mingw32
+++ b/Speed_gcc_i686-w64-mingw32
@@ -1,7 +1,7 @@
 Test speed:
-  naic, no solve      : 5.248e-02 s mean;      52374 us least of  20 runs
-  naic, specamt solve : 9.298e-02 s mean;      92736 us least of  11 runs
-  naic, ee prem solve : 8.541e-02 s mean;      85211 us least of  12 runs
-  finra, no solve     : 2.237e-02 s mean;      22120 us least of  45 runs
-  finra, specamt solve: 5.938e-02 s mean;      59126 us least of  17 runs
-  finra, ee prem solve: 5.520e-02 s mean;      55072 us least of  19 runs
+  naic, no solve      : 5.117e-02 s mean;      50735 us least of  20 runs
+  naic, specamt solve : 9.783e-02 s mean;      91068 us least of  11 runs
+  naic, ee prem solve : 8.398e-02 s mean;      83652 us least of  12 runs
+  finra, no solve     : 2.075e-02 s mean;      20536 us least of  49 runs
+  finra, specamt solve: 5.791e-02 s mean;      57723 us least of  18 runs
+  finra, ee prem solve: 5.379e-02 s mean;      53553 us least of  19 runs
diff --git a/Speed_gcc_x86_64-pc-linux-gnu b/Speed_gcc_x86_64-pc-linux-gnu
index 83a2270..d28751c 100644
--- a/Speed_gcc_x86_64-pc-linux-gnu
+++ b/Speed_gcc_x86_64-pc-linux-gnu
@@ -1,7 +1,7 @@
 Test speed:
-  naic, no solve      : 1.969e-02 s mean;      18873 us least of  51 runs
-  naic, specamt solve : 3.328e-02 s mean;      32799 us least of  31 runs
-  naic, ee prem solve : 3.046e-02 s mean;      30164 us least of  33 runs
-  finra, no solve     : 7.809e-03 s mean;       7328 us least of 100 runs
-  finra, specamt solve: 2.061e-02 s mean;      19933 us least of  49 runs
-  finra, ee prem solve: 1.937e-02 s mean;      18756 us least of  52 runs
+  naic, no solve      : 1.934e-02 s mean;      18402 us least of  52 runs
+  naic, specamt solve : 3.280e-02 s mean;      32060 us least of  31 runs
+  naic, ee prem solve : 3.008e-02 s mean;      29302 us least of  34 runs
+  finra, no solve     : 7.334e-03 s mean;       7103 us least of 100 runs
+  finra, specamt solve: 2.073e-02 s mean;      19476 us least of  49 runs
+  finra, ee prem solve: 1.874e-02 s mean;      18282 us least of  54 runs
diff --git a/cache_file_reads.hpp b/cache_file_reads.hpp
index e4a8390..bcd27a5 100644
--- a/cache_file_reads.hpp
+++ b/cache_file_reads.hpp
@@ -27,6 +27,7 @@
 #include "assert_lmi.hpp"
 
 #include <boost/filesystem/operations.hpp>
+#include <boost/filesystem/path.hpp>
 
 #include <ctime>                        // time_t
 #include <map>
@@ -41,7 +42,8 @@ namespace detail
 /// Motivation: It is costly to deserialize objects from xml, so cache
 /// them for reuse. The cache persists until the program terminates.
 ///
-/// Requires: T::T() and T::T(std::string const& filename).
+/// Requires: T::T(fs::path const& filename).
+///   default ctor is not required
 ///
 /// For each filename, the cache stores one instance, which is
 /// replaced by reloading the file if its write time has changed.
@@ -67,7 +69,7 @@ class file_cache
         return z;
         }
 
-    retrieved_type retrieve_or_reload(std::string const& filename)
+    retrieved_type retrieve_or_reload(fs::path const& filename)
         {
         // Throws if !exists(filename).
         std::time_t const write_time = fs::last_write_time(filename);
@@ -105,7 +107,7 @@ class file_cache
         std::time_t    write_time;
     };
 
-    std::map<std::string,record> cache_;
+    std::map<fs::path,record> cache_;
 };
 } // namespace detail
 
@@ -124,7 +126,7 @@ class cache_file_reads
     /// Postcondition: returned pointer is not null; otherwise,
     /// file_cache::retrieve_or_reload() throws.
 
-    static retrieved_type read_via_cache(std::string const& filename)
+    static retrieved_type read_via_cache(fs::path const& filename)
         {
         return detail::file_cache<T>::instance().retrieve_or_reload(filename);
         }
diff --git a/cache_file_reads_test.cpp b/cache_file_reads_test.cpp
index a91dc43..f82ca3b 100644
--- a/cache_file_reads_test.cpp
+++ b/cache_file_reads_test.cpp
@@ -29,6 +29,7 @@
 #include "timer.hpp"
 
 #include <boost/filesystem/exception.hpp>
+#include <boost/filesystem/path.hpp>
 
 #include <fstream>
 
@@ -36,10 +37,10 @@ class X
     :public cache_file_reads<X>
 {
   public:
-    X() = default;
-    X(std::string const& filename)
+//  X() = default;
+    X(fs::path const& filename)
         {
-        std::ifstream ifs(filename, ios_in_binary());
+        std::ifstream ifs(filename.string(), ios_in_binary());
         istream_to_string(ifs, s_);
         }
 
@@ -72,7 +73,9 @@ class cache_file_reads_test
 void cache_file_reads_test::test_preconditions()
 {
     // X() and X(filename) are required.
-    X x0;
+    // ...well, no, not actually:
+//  X x0;
+    X x0("sample.ill");
     X x1("sample.ill");
 
     // The cache is accessible with or without an object.
diff --git a/dbdict.cpp b/dbdict.cpp
index 377c123..d8f3784 100644
--- a/dbdict.cpp
+++ b/dbdict.cpp
@@ -99,11 +99,11 @@ DBDictionary::DBDictionary()
 /// seem unnecessary, in case the file read by Init() lacks any member
 /// entity (because it's an older version or has been edited, e.g.).
 
-DBDictionary::DBDictionary(std::string const& filename)
+DBDictionary::DBDictionary(fs::path const& filename)
 {
     ascribe_members();
     InitDB();
-    Init(filename);
+    Init(filename.string());
 }
 
 database_entity const& DBDictionary::datum(std::string const& name) const
diff --git a/dbdict.hpp b/dbdict.hpp
index 86057fa..ee76692 100644
--- a/dbdict.hpp
+++ b/dbdict.hpp
@@ -30,6 +30,8 @@
 #include "so_attributes.hpp"
 #include "xml_serializable.hpp"
 
+#include <boost/filesystem/path.hpp>
+
 #include <string>
 
 /// Cached product database.
@@ -45,7 +47,7 @@ class LMI_SO DBDictionary
 
   public:
     DBDictionary();
-    DBDictionary(std::string const& filename);
+    DBDictionary(fs::path const& filename);
 
     ~DBDictionary() override = default;
 
diff --git a/ihs_basicval.cpp b/ihs_basicval.cpp
index d9a9dee..7664a42 100644
--- a/ihs_basicval.cpp
+++ b/ihs_basicval.cpp
@@ -64,7 +64,10 @@
 //============================================================================
 BasicValues::BasicValues(Input const& input)
     :yare_input_         (input)
-    ,product_            (yare_input_.ProductName)
+//  ,product_            (yare_input_.ProductName)
+// Safe to dereference?
+//  ,product_            
(*product_data::read_via_cache(yare_input_.ProductName))
+    ,product_            
(*product_data::read_via_cache(product_data::actual_filename(yare_input_.ProductName)))
     ,database_           (yare_input_)
     ,DefnLifeIns_        {mce_cvat}
     ,DefnMaterialChange_ {mce_unnecessary_premium}
@@ -95,7 +98,9 @@ BasicValues::BasicValues
     // TODO ?? Need loan rate type here?
     )
     :yare_input_         (Input{})
-    ,product_            (a_ProductName)
+//  ,product_            (a_ProductName)
+// Safe to dereference?
+    ,product_            
(*product_data::read_via_cache(product_data::actual_filename(a_ProductName)))
     ,database_
         (a_ProductName
         ,a_Gender
diff --git a/lingo.cpp b/lingo.cpp
index 5bfa449..ee44e8b 100644
--- a/lingo.cpp
+++ b/lingo.cpp
@@ -36,9 +36,9 @@
 
 /// Construct from filename.
 
-lingo::lingo(std::string const& filename)
+lingo::lingo(fs::path const& filename)
 {
-    xml_lmi::dom_parser parser(filename);
+    xml_lmi::dom_parser parser(filename.string());
     xml::element const& root = parser.root_node(xml_root_name());
     int file_version = 0;
     if(!xml_lmi::get_attr(root, "version", file_version))
diff --git a/lingo.hpp b/lingo.hpp
index 6ecd12d..2fe081b 100644
--- a/lingo.hpp
+++ b/lingo.hpp
@@ -28,6 +28,8 @@
 #include "so_attributes.hpp"
 #include "xml_lmi_fwd.hpp"
 
+#include <boost/filesystem/path.hpp>
+
 #include <string>
 #include <unordered_map>
 
@@ -37,7 +39,7 @@ class LMI_SO lingo final
     :public cache_file_reads<lingo>
 {
   public:
-    explicit lingo(std::string const& filename);
+    explicit lingo(fs::path const& filename);
 
     std::string const& lookup(int) const;
 
diff --git a/product_data.cpp b/product_data.cpp
index 2c86472..2496786 100644
--- a/product_data.cpp
+++ b/product_data.cpp
@@ -26,6 +26,7 @@
 
 #include "alert.hpp"
 #include "assert_lmi.hpp"
+#include "ce_product_name.hpp"
 #include "contains.hpp"
 #include "data_directory.hpp"           // AddDataDir()
 #include "map_lookup.hpp"
@@ -118,12 +119,17 @@ std::string const& glossed_string::gloss() const
 }
 
 /// Default ctor, used only by derived classes and friends.
-
 product_data::product_data()
 {
     ascribe_members();
 }
 
+product_data::product_data(fs::path const& filename)
+{
+    ascribe_members();
+    load(filename.string());
+}
+
 /// Construct from product name.
 ///
 /// The argument is a string (typically Input::ProductName) such as
@@ -138,16 +144,17 @@ product_data::product_data()
 /// extensions are added to product names to create file names.
 
 product_data::product_data(std::string const& product_name)
+    :product_data(fs::path(actual_filename(product_name)))
 {
-    ascribe_members();
+}
 
-    fs::path path(product_name);
-    LMI_ASSERT(product_name == fs::basename(path));
-    path = fs::change_extension(path, ".policy");
-    load(AddDataDir(path.string()));
+product_data::product_data(ce_product_name const& product_name)
+    :product_data(fs::path(actual_filename(product_name.str())))
+{
 }
 
 product_data::product_data(product_data const& z)
+// Is this initializer-list necessary?
     :xml_serializable  <product_data> {}
     ,MemberSymbolTable <product_data> {}
 {
@@ -170,6 +177,25 @@ std::string const& product_data::datum(std::string const& 
name) const
     return member_cast<glossed_string>(operator[](name))->datum();
 }
 
+std::string product_data::actual_filename(std::string const& product_name)
+{
+    fs::path path(product_name);
+if(false) warning()
+    << product_name << " product_name\n"
+    << path.string() << " path.string()\n"
+    << fs::basename(path) << " fs::basename(path)\n"
+    << LMI_FLUSH
+    ;
+    LMI_ASSERT(product_name == fs::basename(path));
+    path = fs::change_extension(path, ".policy");
+if(false) warning()
+    << product_name << " product_name\n"
+    << path.string() << " path.string()\n"
+    << LMI_FLUSH
+    ;
+    return AddDataDir(path.string());
+}
+
 /// Reference to named member.
 
 glossed_string& product_data::item(std::string const& name)
diff --git a/product_data.hpp b/product_data.hpp
index d681dbd..d64d1cd 100644
--- a/product_data.hpp
+++ b/product_data.hpp
@@ -25,11 +25,16 @@
 #include "config.hpp"
 
 #include "any_member.hpp"
+#include "cache_file_reads.hpp"
 #include "so_attributes.hpp"
 #include "xml_serializable.hpp"
 
+#include <boost/filesystem/path.hpp>
+
 #include <string>
 
+class LMI_SO_FWD_DECL ce_product_name;
+
 /// A single product datum: a string with an optional gloss.
 ///
 /// For example, the principal string datum might be the full name of
@@ -79,6 +84,7 @@ template<> struct deserialized<product_data>
 class LMI_SO product_data
     :public xml_serializable  <product_data>
     ,public MemberSymbolTable <product_data>
+    ,public cache_file_reads  <product_data>
 {
     friend class BasicValues; // For antediluvian fork only.
     friend class PolicyDocument;
@@ -87,11 +93,15 @@ class LMI_SO product_data
     typedef deserialized<product_data>::value_type value_type;
 
   public:
+    explicit product_data(fs::path const& filename);
     explicit product_data(std::string const& product_name);
+    explicit product_data(ce_product_name const& product_name);
     ~product_data() override;
 
     std::string const& datum(std::string const& name) const;
 
+    static std::string actual_filename(std::string const& product_name);
+
     // Legacy functions to support creating product files programmatically.
     static void write_policy_files();
     static void write_proprietary_policy_files();