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

vine: remove items after iterating hash table #4083

Merged

Conversation

JinZhou5042
Copy link
Member

@JinZhou5042 JinZhou5042 commented Mar 4, 2025

Proposed Changes

For #4081

Merge Checklist

The following items must be completed before PRs can be merged.
Check these off to verify you have completed all steps.

  • make test Run local tests prior to pushing.
  • make format Format source code to comply with lint policies. Note that some lint errors can only be resolved manually (e.g., Python)
  • make lint Run lint on source code prior to pushing.
  • Manual Update: Update the manual to reflect user-visible changes.
  • Type Labels: Select a github label for the type: bugfix, enhancement, etc.
  • Product Labels: Select a github label for the product: TaskVine, Makeflow, etc.
  • PR RTM: Mark your PR as ready to merge.

@JinZhou5042 JinZhou5042 self-assigned this Mar 4, 2025
@dthain
Copy link
Member

dthain commented Mar 4, 2025

Removing things from a hash table inside of an iteration seems like a common operation that should be supported more directly. Is there a good abstraction that we can provide so that it's easy for the caller to get it right?

@Ian2327
Copy link
Contributor

Ian2327 commented Mar 4, 2025

After running blast.py using this fix, the number of valgrind errors are reduced from 411572 to 244766. The 2 major sources of errors are still the same, those involving either vine_manager_choose_resources_for_task or vine_file_replica_table_remove

@btovar
Copy link
Member

btovar commented Mar 5, 2025

Removing things from a hash table inside of an iteration seems like a common operation that should be supported more directly. Is there a good abstraction that we can provide so that it's easy for the caller to get it right?

We could add a delayed delete and then a commit deletes. I'm not sure we can do much better than what @JinZhou5042 did here, though.

@btovar btovar merged commit 41e2e2b into cooperative-computing-lab:master Mar 5, 2025
10 checks passed
@JinZhou5042 JinZhou5042 deleted the remove_after_iterating branch March 5, 2025 14:49
btovar pushed a commit that referenced this pull request Mar 6, 2025
* remove items after iterating hash table

* lint

* use list to collect removable items
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