-
Notifications
You must be signed in to change notification settings - Fork 285
irept: Use singly-linked lists with SUB_IS_LIST [depends on: #3606] #2035
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
src/util/irep_hash_container.cpp
Outdated
| packed.reserve( | ||
| 1+1+sub.size()+named_sub.size()*2+ | ||
| (full?comments.size()*2:0)); | ||
| #ifdef SUB_IS_LIST |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
factor as inline irept::named_sub_size?
|
Perhaps call the define SUB_IS_FORWARD_LIST? |
|
Mentioned in another PR: call it |
3815661 to
9fa4e28
Compare
9fa4e28 to
854f948
Compare
a9d6856 to
f9b4d9e
Compare
allredj
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🚫
This PR failed Diffblue compatibility checks (cbmc commit: a9d6856).
Build URL: https://travis-ci.com/diffblue/test-gen/builds/95396854
Status will be re-evaluated on next push.
Please contact @peterschrammel, @thk123, or @allredj for support.
Common spurious failures:
- the cbmc commit has disappeared in the mean time (e.g. in a force-push)
- the author is not in the list of contributors (e.g. first-time contributors).
The incompatibility may have been introduced by an earlier PR. In that case merging this
PR should be avoided unless it fixes the current incompatibility.
fe52899 to
a20668c
Compare
allredj
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🚫
This PR failed Diffblue compatibility checks (cbmc commit: f9b4d9e).
Build URL: https://travis-ci.com/diffblue/test-gen/builds/95399632
Status will be re-evaluated on next push.
Please contact @peterschrammel, @thk123, or @allredj for support.
Common spurious failures:
- the cbmc commit has disappeared in the mean time (e.g. in a force-push)
- the author is not in the list of contributors (e.g. first-time contributors).
The incompatibility may have been introduced by an earlier PR. In that case merging this
PR should be avoided unless it fixes the current incompatibility.
src/util/irep.cpp
Outdated
|
|
||
| #ifdef NAMED_SUB_IS_FORWARD_LIST | ||
| if( | ||
| i1_sub.size() != i2_sub.size() || |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Move the common i1_sub.size() != i2_sub.size() test outside the block?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, done!
allredj
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🚫
This PR failed Diffblue compatibility checks (cbmc commit: fe52899).
Build URL: https://travis-ci.com/diffblue/test-gen/builds/95400307
Status will be re-evaluated on next push.
Please contact @peterschrammel, @thk123, or @allredj for support.
Common spurious failures:
- the cbmc commit has disappeared in the mean time (e.g. in a force-push)
- the author is not in the list of contributors (e.g. first-time contributors).
The incompatibility may have been introduced by an earlier PR. In that case merging this
PR should be avoided unless it fixes the current incompatibility.
a20668c to
21bc554
Compare
allredj
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🚫
This PR failed Diffblue compatibility checks (cbmc commit: a20668c).
Build URL: https://travis-ci.com/diffblue/test-gen/builds/95402577
Status will be re-evaluated on next push.
Please contact @peterschrammel, @thk123, or @allredj for support.
Common spurious failures:
- the cbmc commit has disappeared in the mean time (e.g. in a force-push)
- the author is not in the list of contributors (e.g. first-time contributors).
The incompatibility may have been introduced by an earlier PR. In that case merging this
PR should be avoided unless it fixes the current incompatibility.
allredj
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
✔️
Passed Diffblue compatibility checks (cbmc commit: 21bc554).
Build URL: https://travis-ci.com/diffblue/test-gen/builds/95404599
irept-related fixes [blocks: #2035]
This reduces the memory footprint by up to 5 (GCC's STL) pointers for both named_sub. The cost is that computing the size of lists and add/remove require additional iterator increments.
As suggested in code review.
21bc554 to
a4828df
Compare
allredj
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
✔️
Passed Diffblue compatibility checks (cbmc commit: a4828df).
Build URL: https://travis-ci.com/diffblue/test-gen/builds/95444945
This reduces the memory footprint by two pointers for both named_sub and
comments. The cost is that computing the size of lists and add/remove require
additional iterator increments.
On Linux/64bit, an irept::dt is now 40 bytes (when SUB_IS_LIST is set) compared to 120 bytes when SUB_IS_LIST is not set.
Do not merge until #1971 is in place.