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

Lazy index #5488

Merged
merged 26 commits into from
Oct 2, 2024
Merged

Lazy index #5488

merged 26 commits into from
Oct 2, 2024

Conversation

zklaus
Copy link
Contributor

@zklaus zklaus commented Sep 17, 2024

Description

Update conda-build to use the new conda.core.index.Index.

Xref conda/conda#13880
Xref #5154

Checklist - did you ...

  • Add a file to the news directory (using the template) for the next release's release notes?
  • Add / update necessary tests?
  • Add / update outdated documentation?

@conda-bot conda-bot added the cla-signed [bot] added once the contributor has signed the CLA label Sep 17, 2024
Copy link

codspeed-hq bot commented Sep 17, 2024

CodSpeed Performance Report

Merging #5488 will improve performances by ×26

Comparing zklaus:lazy-index (f21952d) with main (6f20ff7)

Summary

⚡ 1 improvements
✅ 4 untouched benchmarks

Benchmarks breakdown

Benchmark main zklaus:lazy-index Change
test_get_build_index 1,633 ms 63.3 ms ×26

@kenodegard
Copy link
Contributor

kenodegard commented Sep 25, 2024

@zklaus we currently don't have any memray tests hence the memray CI job fails, do we have a test to add here or should we remove the memray CI job?

@kenodegard kenodegard marked this pull request as ready for review September 25, 2024 15:53
@kenodegard kenodegard requested a review from a team as a code owner September 25, 2024 15:53
@beckermr
Copy link
Contributor

@zklaus @kenodegard @jaimergp Do we not have the proper benchmark to see performance improvements from this change? Should we write one?

@kenodegard
Copy link
Contributor

@beckermr I agree adding a benchmark to see the improvement would be great. I'll take a look and see if I can come up with something simple.

@kenodegard kenodegard mentioned this pull request Oct 1, 2024
3 tasks
@kenodegard
Copy link
Contributor

@beckermr I think #5508 will do the trick, I've tested this locally and saw a 10x speedup, let's see what CodSpeed says once we merge the test into main

@zklaus
Copy link
Contributor Author

zklaus commented Oct 2, 2024

Regarding memray, it would be nice to have a similar reading about possible memory usage improvements. I'll see if I can add something simple to #5508 quickly.

@kenodegard
Copy link
Contributor

@zklaus unfortunately #5508 has already been merged, can we instead work on adding the memray CI workflow and the associated memray test in a separate PR?

@kenodegard kenodegard merged commit 0ef3749 into conda:main Oct 2, 2024
28 checks passed
@bgruening
Copy link

Could we cut a new release please with this change? We have memory problems on Azure 😔

@jaimergp
Copy link
Contributor

I guess this will be included in 24.11.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla-signed [bot] added once the contributor has signed the CLA
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

7 participants