Re: [lmi] [lmi-commits] [6323] Use unique_filepath() for all PDF and spreadsheet output

[Vadim Zeitlin]

On Tue, 06 Oct 2015 15:17:19 +0000 Greg Chicares <address@hidden> wrote:

GC> Modified: lmi/trunk/emit_ledger.cpp
GC> ===================================================================
GC> --- lmi/trunk/emit_ledger.cpp       2015-10-06 13:26:42 UTC (rev 6322)
GC> +++ lmi/trunk/emit_ledger.cpp       2015-10-06 15:17:19 UTC (rev 6323)
...
GC> @@ -55,9 +56,15 @@
GC>      (fs::path const& case_filepath
GC>      ,mcenum_emission emission
GC>      )
GC> -    :case_filepath_ (case_filepath)
GC> -    ,emission_      (emission)
GC> -{}
GC> +    :case_filepath_              (case_filepath)
GC> +    ,tsv_ext_                    
(configurable_settings::instance().spreadsheet_file_extension())
GC> +    ,case_filepath_spreadsheet_  (unique_filepath(case_filepath,           
  tsv_ext_))
GC> +    ,case_filepath_group_roster_ (unique_filepath(case_filepath, ".roster" 
+ tsv_ext_))
GC> +    ,case_filepath_group_quote_  (unique_filepath(case_filepath, 
".quote.pdf"        ))
GC> +    ,emission_                   (emission)
GC> +{
GC> +    LMI_ASSERT(!case_filepath_.empty());
GC> +}

 Hello,

 I have a small problem with this change as it always initializes
case_filepath_xxx_ objects, even when we don't need any of them -- and we
never need all of them at once. This is not actually harmful, of course,
but it's still a gratuitous performance pessimization and while it probably
isn't normally noticeable in practice, unique_filepath() is still an
expensive function to call as it does at least one and possibly 3 file
operations. And I could see it becoming noticeable e.g. when using a slow
network disk[*].

 So ideally I'd only initialize these paths on demand, i.e. when they're
first needed. Would it be useful to make a patch doing this?

 Please let me know,
VZ

[*] I'm afraid I'm showing my age here, it's supposed to be called "the
    cloud" now I believe.