Re: [lmi] [PATCH] Micro-optimization for ledger_format() taking vector

[Greg Chicares]

On 2017-06-29 16:16, Vadim Zeitlin wrote:
> 
>  While looking at this code, I couldn't help noticing that ledger_format()
> makes a copy of all vectors passed to it, which seems completely
> unnecessary -- even if these vectors are not exactly huge (limited to ~100
> elements, I think), there is a number of them and just avoid a ~100
> allocations is, I believe, worth it, especially when it can be avoided by
> simply passing the vector by const reference instead of by value, so I went
> ahead and made this change and also another small one inside the function
> itself to avoid more memory (re)allocations there, please see
> 
>       https://github.com/vadz/lmi/pull/58/files

I had never previously thought of doing this:

$wget 
https://github.com/vadz/lmi/commit/24de308661afb879e7319918840648bddb96071b.patch
$wget 
https://github.com/vadz/lmi/commit/ab079b23a41dfc7082e0b643d868f3ddb5438ef5.patch

but it works, and prevents mozilla from mangling patches.

>  This is not urgent at all, but OTOH it's a very simple change and so it
> shouldn't take you long to merge it.

Thanks. This command:

$grep '[(,].*std::vector' *.hpp |sed -e '/&/d' 2>&1 |less -S -N

finds the original
  ledger_text_formats.hpp:    (std::vector<double>               dv
but doesn't find any similar problem; I hope that means this sort
of gratuitous mistake is rare.

Is there any reason not to s/push_back/emplace_back/ here?

More generally, looking at even the much-improved code (pushed a
moment ago):

std::vector<std::string> ledger_format
    (std::vector<double> const&        dv
    ,std::pair<int,oenum_format_style> f
    )
{
    std::vector<std::string> sv;
    sv.reserve(dv.size());
    for(auto const& i : dv)
        {
        sv.push_back(ledger_format(i, f));
        }
    return sv;
}

makes me pine for APL, where we'd just write something like
  (i f)ledger_format¨dv
to apply the scalar version of ledger_format() to each ("¨")
element of vector 'dv', returning an array of strings.