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

Reduce memory footprint of culling P2P rechunking #8845

Conversation

hendrikmakait
Copy link
Member

@hendrikmakait hendrikmakait commented Aug 29, 2024

Previously, culling appeared to have an exponentially-growing memory footprint, this PR fixes that.

  • Tests added / passed
  • Passes pre-commit run --all-files

Copy link
Contributor

github-actions bot commented Aug 29, 2024

Unit Test Results

See test report for an extended history of previous test failures. This is useful for diagnosing flaky tests.

    25 files  ±0      25 suites  ±0   10h 15m 38s ⏱️ - 6m 8s
 4 122 tests ±0   4 002 ✅  - 2    111 💤 ±0   9 ❌ +2 
47 615 runs  ±0  45 498 ✅  - 3  2 107 💤 ±0  10 ❌ +3 

For more details on these failures, see this check.

Results for commit 4db28ae. ± Comparison against base commit 4aeed40.

♻️ This comment has been updated with latest results.

@hendrikmakait hendrikmakait marked this pull request as ready for review August 29, 2024 18:56
@jrbourbeau jrbourbeau mentioned this pull request Aug 29, 2024
5 tasks
Copy link
Member

@jrbourbeau jrbourbeau left a comment

Choose a reason for hiding this comment

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

Thanks @hendrikmakait, I can confirm this drastically reduces the memory required to kick off some of the large ERA5 rechunking jobs I've been running lately. (@dcherian you may also find this one interesting)

@dcherian
Copy link

you guys are killing it :)

@hendrikmakait
Copy link
Member Author

Benchmarking on Coiled showed no impact on performance (https://github.com/coiled/benchmarks/actions/runs/10628454825). Note that we do not measure client-side memory.

@phofl phofl merged commit e19c630 into dask:main Aug 30, 2024
22 of 32 checks passed
@phofl
Copy link
Collaborator

phofl commented Aug 30, 2024

thanks!

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.

4 participants