Skip to content

Lock rank cleanup, part 1#9524

Open
andyleiserson wants to merge 4 commits into
gfx-rs:trunkfrom
andyleiserson:jj-push-quyz
Open

Lock rank cleanup, part 1#9524
andyleiserson wants to merge 4 commits into
gfx-rs:trunkfrom
andyleiserson:jj-push-quyz

Conversation

@andyleiserson
Copy link
Copy Markdown
Contributor

Progress towards #5937.

Does the following, in separate commits for each top-level bullet:

  • Improvements to the lock analyzer
    • Groups the graph nodes by non-leaf, connected leaf, and disconnected leaf, instead of only by leaf and non-leaf.
    • List followers in the same order as the ranks themselves.
    • Don't print the acquisition locations by default. This makes the output closer to what appears in rank.rs.
  • Release COMMAND_BUFFER_DATA before encoding commands. (This may sound dubious, but it's not. COMMAND_BUFFER_DATA is just protecting the command buffer mutable state. It is acquired just before encoding commands to move the mutable state out of the lock-protected location entirely. Once that is done, the mutable state only holds the empty CommandEncoderStatus::Consumed sentinel. There aren't any hazards between the sentinel and the encoding process itself.)
  • Print more detail for lock release order violations
  • Use unique lock ranks for concurrently-accessed registries.

Testing
It is hard to use the lock validation build incrementally for each of the changes, but at the tip of my branch, the lock validation build can pass our test suite (including the enabled CTS tests).

Squash or Rebase? Rebase

Checklist

  • I self-reviewed and fully understand this PR.
  • WebGPU implementations built with wgpu may be affected behaviorally.
  • Validation and feature gates are in place to confine behavioral changes.
  • Tests demonstrate the validation and altered logic works.
    • See above.
  • CHANGELOG.md entries for the user-facing effects of this change are present.
    • Maybe one changelog entry after all the changes, mentioning deadlocks have been resolved?
  • The PR is minimal, and doesn't make sense to land as multiple PRs.
    • Can be further split by commits if desired.
  • Commits are logically scoped and individually reviewable.
  • The PR description has enough context to understand the motivation and solution implemented.

* Groups the graph nodes by non-leaf, connected leaf, and disconnected
  leaf, instead of only by leaf and non-leaf.
* List followers in the same order as the ranks themselves.
* Don't print the acquisition locations by default. This makes the
  output closer to what appears in `rank.rs`.
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.

1 participant