To give you a quick feedback: thank you for your report, this is a bug, I see where it is, expect a fix soon.
(TWR incorrectly updates internal unit price)

Should be fixed by #2300

Thank you for the quick reaction and fix! I can confirm that the numbers with #2300 look much better.

Unfortunately the proposed fix also makes my yearly ROI report 7-8x slower. That's better than wrong data, but somewhat painful.

Unfortunately the proposed fix also makes my yearly ROI report 7-8x slower

I do see slowdown, but not as drastic as you.

Trying my dataset:

v1.32.99 :  13.5-14s
v1.40 : 14-14.5s
v1.41.99 (current HEAD) : 15s
v1.41.99+#2300 : 25s

This is ... weird. I'll try and see why it is so slow, change certainly does not look like something that should have a pronounced effect

@TPolzer i've pushed something into #2300 that should hopefully lead to more sharing of intermediate results and recover some of the lost performance. Can you please see how this behaves for you?

For me this brings runtimes back to v1.40 levels

Tried it just now, I don't see a significant change in performance, it's still pretty slow.

Trying with a subset of my roi query and some debug output:
old:

$ time hledger roi [...] --value then,CHF --begin 2018 --end tomorrow -Y --debug=2 |& grep seeking | sort | uniq -c
     15 seeking EUR to CHF price using forward prices
      9 seeking IE00B4L5Y983 to CHF price using forward prices
[...]

real	0m0.760s
user	0m0.731s
sys	0m0.035s

vs. new:

$ time hledger roi [...] --value then,CHF --begin 2018 --end tomorrow -Y --debug=2 |& grep seeking | sort | uniq -c
     15 seeking EUR to CHF price using forward prices
   2154 seeking IE00B4L5Y983 to CHF price using forward prices
[...]

real	0m15.982s
user	0m15.246s
sys	0m0.863s

The 15 EUR lookups are cashflow (in EUR) transactions I think, and they're the same. It should need the "IE00B4L5Y983 to CHF" chain only at period boundaries and cashflow events, too?

and ond dates you have P directives too (which was not happening earlier)

Yeah... that explains the dramatic difference in runtime, since I have a large automatically created list of prices.

But that's not actually necessary, is it? To compute TWR it's sufficient to compute prices at period boundaries and cashflow events. E.g. in the initial example on this bug, inserting a lot of fine grained P directives would not change the result at all.

Just to double-check, in the example above is "old" a v1.40, or v1.41.99 without #2300?

"old" is actually 1.32.3 from Debian.

How many P directives have you @TPolzer ? I'm thinking we might need to mention that in general docs.

I'm storing daily FX rates for three currencies and weekly rates for a handful of investments. I should probably reduce the latter to monthly rates. My current auto generated price list has ~21k entries.

I understand that general parsing etc scales linearly, but I was under the impression (so far I think correctly) that most analysis cost should be unaffected by the number of P statements.

It's good to know the number. If you're seeing no slowdowns in most features, with 20k prices, that's good - perhaps it's something suboptimal in roi then.