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.

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

Lin Out_channel shrink cleanup #387

Merged
merged 11 commits into from
Sep 1, 2023
Merged

Lin Out_channel shrink cleanup #387

merged 11 commits into from
Sep 1, 2023

Conversation

jmid
Copy link
Collaborator

@jmid jmid commented Aug 30, 2023

This PR contains a range of adjustments to address the long shrinking sequences reported in #378:

  • bf61729 and d5ff92a disable char element shrinking inside bytes and strings for the test
  • 2433827 removes the lift combinator to reduce allocations
  • ba20a6f adjusts the cmd frequencies
  • 6c8db20 drops flush_all entirely
  • Finally e1a1de3 reduces overall Lin allocations during check_seq_cons and should thus benefit other Lin tests too.

Overall, this makes the test behaviour reasonable. Below I include a CI summary table from running 20 focused tests across the 2 CI systems (I note runs taking more than 1000s as "outliers"). Observations:

  • There's a trend of more outliers+timeouts after 5.0 - extra GC activity on +5.1.0 has been confirmed with perf and olly gc-stats measurements
  • Disabling the test on macOS should be kept for now, an explanation would be nice though
  • There was a crazy linux 5.1 debug 12514.3s outlier!

Overall the PR offers improvements, not the final word. They should help reduce false CI alarms though.

The last two commits 460b97c and f7223d6 update the plain Lin Out_channel test to move it closer feature-wise to the Lin DSL version. These are steps towards creating an standalone reproducer for the extra +5.1.0 GC activity.

Target Outliers Timeout Remarks
Linux 5.1 -
Linux trunk 1973.0s
Linux 5.1 debug 12514.3s 12th run
Linux trunk debug 1462.0s 2350.7s
32bit trunk -
32bit 5.1 -
Bytecode 5.1 -
Bytecode trunk -
linux-arm64-5.0 -
linux-arm64-5.1 2367.5s 1150.1s 1360.5s 3558.0s
linux-arm64-5.2 1426.0s 1416.6s 1435.6s 1160.6s
linux-s390x-5.1 1493.5s
linux-s390x-5.2 -
linux-ppc64le-5.2 2497.0s 1434.2s 3345.5s 3099.9s 1852.6s 2503.3s 11th run
macOS 5.1 7728.8s 1371.7s 3rd run
macOS trunk - 2nd run
macos-arm64-5.0 2090.3s
macOS-arm64-5.1 3017.1s 3278.3s 2001.6s 1961.0s 1866.6s 2803.1s 10th run 6/9 w/no counterexample!
macOS-arm64-5.2 3988.2s 3447.5s 3344.8s 2651.1s 3579.0s 6th run 5/5 w/no counterexample!
Windows 5.1 1030.1s 2523.1s
Windows trunk 2471.4s 1139.8s 2522.6s
Win bytecode 5.1 1391.8s 3730.3s
Win bytecode trunk -
Cygwin 5.1 1004.0s 1206.1s
Cygwin trunk 1010.6s

@jmid
Copy link
Collaborator Author

jmid commented Aug 30, 2023

Oh wow 😮 linux-arm64 5.2 had a 16443.2s Lin DSL Out_channel test with Domain run, yet somehow still managed to complete within the timeout limit!

Copy link
Collaborator

@shym shym left a comment

Choose a reason for hiding this comment

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

Looks good to me! :-)

lib/lin.ml Outdated Show resolved Hide resolved
@jmid
Copy link
Collaborator Author

jmid commented Aug 31, 2023

CI summary for f7223d6 from yesterday:

For the Cygwin failures, I get suspicious that the slowness is affecting the ability to trigger test failures and thereby cause needlessly long shrink runs 🤔

@jmid
Copy link
Collaborator Author

jmid commented Sep 1, 2023

CI summary for bb94d34:

So 3/23 failures - but none of them being false positives! 😃

@jmid
Copy link
Collaborator Author

jmid commented Sep 1, 2023

CI summary for b9509bb

So, 4/52 failures with only 1 of them being noise (Cygwin slowness is a genuine issue) and 1 being container related.

I'll merge 😃

@jmid jmid merged commit 29eb834 into main Sep 1, 2023
44 of 47 checks passed
@jmid jmid deleted the outchannel-shrink-cleanup branch September 1, 2023 12:38
@jmid jmid mentioned this pull request Jan 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants