deprecates exprt::move_to_operands(...) #3073
Conversation
kroening
requested review from
peterschrammel,
pkesseli and
smowton
as code owners
There was a problem hiding this comment.
Sorry, a bit of a silly request-for-changes: could you please copy your analysis details from the other PR into the commit message? I think it's worth preserving that for the future, and it will be rather hard to find as this is across different PRs. Thank you!!!
There was a problem hiding this comment.
Passed Diffblue compatibility checks (cbmc commit: d9d3428).
Build URL: https://travis-ci.com/diffblue/test-gen/builds/86459004
Since sharing has been introduced there is no measureable performance
benefit when using move_to_operands compared to copying:
for(int i=0; i<2000000; i++)
{
irep_idt id;
symbol_exprt s(id, bool_typet());
exprt something;
something.move_to_operands(s);
//something.copy_to_operands(s);
}
with move_to_operands: 0.59s
with copy_to_operands: 0.55s
This does not appear to require a recent compiler. Thus, there is no reason
to believe that moving ireps would be any faster than copying. Moving has
the additional disadvantage that there is a side-effect on the object that
is moved.
A scenario in which moving offers a benefit is the idiom in which
1) an irep x is copied,
2) the copy is then subsequently modified,
3) x is then destroyed.
The proposal is to do step 1) via an rvalue reference and std::move()
instead.
kroening
force-pushed
the
deprecate_move_to_operands
branch
from
d9d3428 to
6f4274d
Compare
|
@tautschnig Done. |
There was a problem hiding this comment.
Passed Diffblue compatibility checks (cbmc commit: 6f4274d).
Build URL: https://travis-ci.com/diffblue/test-gen/builds/87094284
|
Given the discussion in the code_blockt::add / move PR, quite surprised this got merged. There is only no performance difference when using copy doesn't cause any extra CoW breaks; would prefer to revert this and use move judiciously. |
Since sharing has been introduced there is no measureable performance
benefit when using move_to_operands compared to copying. Moving has the
disadvantage that there is a side-effect on the object that is moved.