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

tests: group tests into several link-farms (10 tests per farm) #2041

Merged
merged 4 commits into from
Aug 19, 2024

Conversation

MattSturgeon
Copy link
Member

@MattSturgeon MattSturgeon commented Aug 19, 2024

  • tests/lib-test: correctly print expected/result
  • lib/util: add groupListBySize
  • tests: group tests by 10
  • flake-modules/devshell: update tests command with link-farm support

In an attempt to strike a balance between having one very intensive test and many smaller tests at the checks.<system>.* level, this PR groups the tests into bundles of 10, currently resulting in 28 checks.<system>.test-* link farms and 46 total checks.

Quoting #2015 (comment):

On the old hardware we were using 16 workers for eval, now we are using 32. Only splitting the linkfarm up a bit (e.g. into 4) likely won't help much as they will still end up being evaluated at the same time but by more than that or by path should work.

We can always dial the group size up/down to find the right balance, but 28 groups of 10 feels like a good starting point (112 groups of 10, when multiplied by the 4 systems).

As per #1878 (comment), we can test memory usage locally using:

nix run nixpkgs#nix-eval-jobs -- --workers 1 --check-cache-status --force-recurse --flake .#checks

cc @zowoq
cc @GaetanLepage

@traxys I've had to re-work some details of your tests shell script, supplying it with a pre-generated tests.json file, which looks like (full file: tests.json):

{
  "examples": "test-1.passthru.entries.examples",
  "extended-lib": "test-1.passthru.entries.extended-lib",
  "modules-clipboard": "test-1.passthru.entries.modules-clipboard",
  "modules-commands": "test-1.passthru.entries.modules-commands",
  "modules-editorconfig": "test-1.passthru.entries.modules-editorconfig",
  "modules-files": "test-1.passthru.entries.modules-files",
  "modules-filetypes": "test-2.passthru.entries.modules-filetypes",
  "modules-keymaps": "test-2.passthru.entries.modules-keymaps",
  "modules-lua-loader": "test-2.passthru.entries.modules-lua-loader",
  "modules-options": "test-2.passthru.entries.modules-options",
  "plugins-bufferlines-barbar": "test-3.passthru.entries.plugins-bufferlines-barbar",
  "plugins-bufferlines-bufferline": "test-3.passthru.entries.plugins-bufferlines-bufferline",
  "plugins-colorschemes-base16": "test-3.passthru.entries.plugins-colorschemes-base16",
  "plugins-colorschemes-catppuccin": "test-3.passthru.entries.plugins-colorschemes-catppuccin",
}

I think doing this via passthru avoids IFD, but I'm unsure how much cost this has added to evaluating/building the devshell.

Note

When using the tests command, you should no-longer pass a "test-" prefix.
I figured the prefix was redundant in the context of passthru.entries.

In some edge cases, expected and result can't be directly used in string
interpolation, so pass them through `lib.generators.toPretty` to be safe.
Splits up a list into many sub-lists based on the given max-size.

e.g.
```nix
groupListBySize 2 [ 1 2 3 4 5 ]
=> [ [ 1 2 ] [ 3 4 ] [ 5 ] ]
```
Attempt to find a middle ground between having a single link-farm and
one test per file.

This groups up to ten tests per (roughly) 28 link-farms.
Since we're using link-farms, we need to access the underlying tests as
`passthru.entries.*`.
@MattSturgeon
Copy link
Member Author

Initial feedback from buildbot is good: built in 4.5m, compared with the last few builds which took ~1h:

image

Obviously not statistically significant, since this is only one build.

@khaneliman
Copy link
Contributor

khaneliman commented Aug 19, 2024

Initial feedback from buildbot is good: built in 4.5m, compared with the last few builds which took ~1h:
Obviously not statistically significant, since this is only one build.

I think it was also faster at this time of day during the last few PR's that I built. Might be good to look at what the runs look like from similar time periods.

These are from around this time of night a few days ago. So it would still be several times faster.
image

@MattSturgeon
Copy link
Member Author

Running nix-eval-jobs on this branch seems to ramp up memory and then suddenly gc, I'm not sure the best way to measure RAM usage, but just observing my total system RAM usage, it seems to ramp up to a max of around 12GB (sometimes less) before dropping back down to around 5-6GB, then slowly ramping up again.

image

This isn't very scientific though, because I have several browser tabs (etc) open in the background.

@zowoq
Copy link
Contributor

zowoq commented Aug 19, 2024

I ran the buildbot job again a couple of times and I've ran just the nix-eval-jobs command on the CI host a few times, it all looks good. Thank you for working on this.

Copy link
Contributor

@khaneliman khaneliman left a comment

Choose a reason for hiding this comment

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

From the bits that I evaluated, I could see marginal memory footprint increase locally but the performance in buildbot looks magnitudes better. Everything seems to function fine on my desktop and laptop (x86 linux and arm darwin). As long as it looks alright for team maintaining buildbot it seems to be a good change.

@MattSturgeon

This comment was marked as resolved.

This comment was marked as resolved.

@mergify mergify bot merged commit 312db6b into nix-community:main Aug 19, 2024
4 checks passed
@mergify mergify bot temporarily deployed to github-pages August 19, 2024 07:38 Inactive
@MattSturgeon MattSturgeon deleted the group_tests branch August 19, 2024 07:38
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.

3 participants