Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Improve memory footprint of some test #4696

Merged
merged 24 commits into from
Oct 25, 2023
Merged

Improve memory footprint of some test #4696

merged 24 commits into from
Oct 25, 2023

Conversation

coot
Copy link
Contributor

@coot coot commented Oct 20, 2023 •
edited
Loading

Description

Replaced implementation of prop_shrink_nonempty with prop_shrinkCarefully since the latter can shrink.
Improved memory footprint of prop_shrink_nonempty, e.g. from

1,825,176,616 bytes maximum residency (105015 sample(s))

to

1,147,232 bytes maximum residency (4062 sample(s))

as measured with on the shrink nonequal GovernorMockEnvironment test.

  • ouroboros-network-testing: ShrinkCarefully
  • ouroboros-network-testing: improved prop_shrink_nonequal
  • peer-selection-test: nightly prop_shrink_nonequal_GovernorMockEnvironment

Checklist

  • Branch
    • Updated changelog files.
    • Commit sequence broadly makes sense
    • Commits have useful messages
    • The documentation has been properly updated
    • New tests are added if needed and existing tests are updated
    • If serialization changes, user-facing consequences (e.g. replay from genesis) are confirmed to be intentional.
  • Pull Request
    • Self-reviewed the diff
    • Useful pull request description at least containing the following information:
      • What does this PR change?
      • Why these changes were needed?
      • How does this affect downstream repositories and/or end-users?
      • Which ticket does this PR close (if any)? If it does, is it linked?
    • Reviewer requested

@coot
Copy link
Contributor Author

coot commented Oct 20, 2023

Related to #4687.

@coot coot requested a review from bolt12 October 20, 2023 10:46
@coot coot mentioned this pull request Oct 20, 2023
Open
@coot coot added the testing label Oct 20, 2023
@coot
Copy link
Contributor Author

coot commented Oct 20, 2023

Improved ouroboros-network-framework:sim-tests (mostly Server2 tests) from:

image

to

image

There are two more tests that can be improved:

  • prop_connection_manager_counters
  • prop_timeouts_enforced

@coot coot force-pushed the coot/test-utils-improvements branch from f180ed5 to 1a823a8 Compare October 20, 2023 15:52
@coot coot requested a review from a team as a code owner October 20, 2023 19:26
@coot coot force-pushed the coot/test-utils-improvements branch from 00e8458 to 4348027 Compare October 20, 2023 19:36
@coot
Copy link
Contributor Author

coot commented Oct 21, 2023

The improvement in input-output-hk/typed-protocols#43 is quite significant:
from:
image
to
image

bolt12
bolt12 reviewed Oct 23, 2023
View reviewed changes
Copy link
Contributor

@bolt12 bolt12 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, unfortunate how much of the memory increases just by having a call to counterexample and the test not failing

ouroboros-network-framework/sim-tests/Test/Ouroboros/Network/Server2/Sim.hs Outdated Show resolved Hide resolved
ouroboros-network-framework/sim-tests/Test/Ouroboros/Network/Server2/Sim.hs Outdated Show resolved Hide resolved
@coot coot force-pushed the coot/test-utils-improvements branch from 2e05773 to 3ba43a4 Compare October 23, 2023 09:38
@coot
Copy link
Contributor Author

coot commented Oct 23, 2023

unfortunate how much of the memory increases just by having a call to counterexample and the test not failing

This is unavoidable. It would be cool to have an option to rerun the failed tests with a debug option to capture the trace.

@coot coot mentioned this pull request Oct 23, 2023
Merged
9 tasks
coot added 16 commits October 25, 2023 14:35
@coot
ouroboros-network-testing: ShrinkCarefully
55bef14 
Replaced the implementation of `prop_shrink_nonempty` with `prop_shrinkCarefully`.
Use `ShrinkCarefully` in `prop_shrink_valid`.
@coot
ouroboros-network-testing: improved prop_shrink_nonequal
a00e9a7 
Improved memory footprint of `prop_shrink_nonequal`. For large test
cases retaining whole shrink list leads to a large memory leak
(retaining ~2GB of data).
@coot
peer-selection-test: nightly prop_shrink_nonequal_GovernorMockEnviron…
b7328a6 
…ment

The property is not essential, can be tested in nightly runs only.
@coot
server-tests: syntax
7859aed
@coot
server-tests: prop_connection_manager_pruning
1b3a9fa 
Analyse the `Trace` as a stream.  This reduces the memory footprint from
35MB to 12MB.
@coot
server-tests: validate_transitions
5b1f200 
Reduced memory footpring from 21MB to 10MB.
@coot
server-tests: prop_connection_manager_counters & prop_timeouts_enforced
52adeec 
Added a TODOs.
@coot
server-tests: refactor prop_inbound_governor_prunning
65a1b58 
By analysing the data as a single stream reduced the memory footprint
from 35MB to 12MB.  We won't log the simulation trace on error.  This
should not be a problem since `io-sim` tests are deterministic, and
hence reproducible.
@coot
server-tests: prop_inbound_governor_valid_transition_order
1c19666 
Improved memory footprint (from 21MB to 8MB).
@coot
ouroboros-network-protocols: removed unused GHC extensions
6687858
@coot
ouroboros-network-protocols: imporoved performance of handshake tests
bbe9b8c
@coot
Use typed-protocols-0.1.1
65b5992
@coot
Updated hackageNix
2fd8295 
Use `ghc963`.
@coot
compile bitvec with -simd flag
f4dac7d
@coot
network-mux: allow to use RTS options in mux tests
e6456da
@coot
server-tests: code style
757aa73
coot added 4 commits October 25, 2023 14:35
@coot
connection-manager: bug fix
010d9b2 
When a connection is terminating, we can accept a new connection, but we
need to be careful to create a new `MutableConnState` for it, otherwise
the new connection will be removed from connection manager map leading
to a leaked resource, and subsequent assertion failures of the inbound
governor (e.g. when it will execute `promotedToWarmRemote`).
@coot
inbound-governor: fixed assertion
de110fd
@coot
snocket: derive TestAddress Show instance using Quiet
d56c9d4
@coot
sim-net: existential ResourceException
908d837 
Hide `addr`, which makes it more robust to catch the exception.
@coot coot force-pushed the coot/test-utils-improvements branch from 4e3f226 to cb3894b Compare October 25, 2023 12:35
bolt12
bolt12 reviewed Oct 25, 2023
View reviewed changes
ouroboros-network-framework/sim-tests/Test/Ouroboros/Network/Server2/Sim.hs
| NoActiveConnection addr addr
data ExperimentError =
forall addr. (Typeable addr, Show addr) => NodeNotRunningException addr
-- | forall addr. (Typeable addr, Show addr) => NoActiveConnection addr addr
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I left it, so we can come back to it at some point.

@coot coot force-pushed the coot/test-utils-improvements branch from 3bf9a76 to 9a8ba69 Compare October 25, 2023 19:20
@coot coot enabled auto-merge October 25, 2023 19:49
@coot coot force-pushed the coot/test-utils-improvements branch from 9a8ba69 to d60f84a Compare October 25, 2023 19:52
@coot coot added this pull request to the merge queue Oct 25, 2023
Merged via the queue into master with commit 3b63ca5 Oct 25, 2023
7 of 8 checks passed
@coot coot deleted the coot/test-utils-improvements branch October 25, 2023 20:55
@coot coot self-assigned this Oct 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bolt12 bolt12 bolt12 approved these changes

Assignees

@coot coot

Labels
testing
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@coot @bolt12