lmi
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Vadim Zeitlin
Subject: Re: [lmi] [lmi-commits] [6323] Use unique_filepath() for all PDF and spreadsheet output
Date: Wed, 7 Oct 2015 15:25:50 +0200

On Wed, 07 Oct 2015 01:17:39 +0000 Greg Chicares <address@hidden> wrote:

GC> I'm not sure we should worry much about the less common case where one
GC> (or more) of these three files is open in a viewer that locks it. In
GC> that case, we make three filesystem calls:
GC>   boost::filesystem::exists()
GC>   boost::filesystem::remove()
GC>   boost::filesystem::exists()
GC> How slow can they be? Do they really add more overhead than the
GC> (unfortunately necessary) try-catch?

 Absolutely yes to the latter. Exception handling in C++ is slow compared
to normal code execution because it can involve executing hundreds or even
thousands of machine instructions, but it's still orders of magnitude
faster than accessing external storage.

 Ah, and just to give you the reason number 12657 for upgrading lmi
toolchain: this try-catch wouldn't be necessary at all with non-ancient
boost versions that provide overloads of all functions returning an
error_code instead of throwing an exception. Of course, they're not present
in our ur-ancient version of the library...


GC> >  So ideally I'd only initialize these paths on demand, i.e. when they're
GC> > first needed. Would it be useful to make a patch doing this?
GC> 
GC> Let's see...we could move everything into the body of the ctor,
GC> and initialize each path conditionally:
GC> 
GC>     if(emission_ & mce_emit_spreadsheet)
GC>         {
GC>         case_filepath_spreadsheet_ = unique_filepath(case_filepath, 
tsv_ext_))
GC>         }
GC>     if(emission_ & something else ...)
GC>         { ... and so on
GC> 
GC> If that's all, then...okay, I could sacrifice that much terseness in
GC> return for removing even a theoretical problem, and I'd apply a patch
GC> that does only that and little to nothing more.

 This is indeed a good solution which is even simpler than what I had in
mind, so here is the patch doing just this. I've tested that it works
correctly for the group quote output, i.e. produces the correct file
initially, overrides it if possible and creates a file with a "unique" name
otherwise.

 Please notice that this patch is independent of my other changes and it
should be possible to apply it either before or after them.

 Thanks,
VZ

Attachment: 0001-Only-initialize-the-unique-paths-emit_ledger-really-.patch
Description: Text document


reply via email to

[Prev in Thread] Current Thread [Next in Thread]