Skip to content

Conversation

@Seven-Streams
Copy link
Collaborator

@Seven-Streams Seven-Streams commented Nov 7, 2025

This PR fixes a concurrency problem in the optimization for TagDispatch, and fixes a bug in SubGrammarAdder.

Copilot AI review requested due to automatic review settings November 7, 2025 08:00
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR fixes a concurrency issue in the TagDispatch optimization by converting a class member variable to a local variable that is passed by reference to the optimization function. This eliminates potential race conditions when multiple threads access the optimization data structure.

Key Changes:

  • Moved the tag_dispatch_rule_id_to_second_slicing_bitset from a class member to a local variable
  • Extracted the TagDispatch optimization logic into a separate method that accepts the map by pointer parameter
  • Added documentation for the new TagDispatchOptimization method

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@Seven-Streams Seven-Streams force-pushed the fix/2025-11-07/tag_optim branch from a86d04d to 674f2f4 Compare November 9, 2025 08:53
@Seven-Streams Seven-Streams changed the title [Fix] Fix the optimization for TagDispatch. [Fix] Fix the optimization for TagDispatch and SubGrammarAdder. Nov 9, 2025
Copy link
Collaborator

@Ubospica Ubospica left a comment

Choose a reason for hiding this comment

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

Please see if you can reproduce it, even though the running time of the test is large. We can consider putting heavy tests in a daily efficiency test.

Copy link
Collaborator

@Ubospica Ubospica left a comment

Choose a reason for hiding this comment

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

Please see if you can reproduce it, even though the running time of the test is large. We can consider putting heavy tests in a daily efficiency test. It's fine if it's too difficult or complex to do so.

Signed-off-by: Yuchuan <[email protected]>
Signed-off-by: Yuchuan <[email protected]>
Signed-off-by: Yuchuan <[email protected]>
Signed-off-by: Yuchuan <[email protected]>
Signed-off-by: Yuchuan <[email protected]>
Signed-off-by: Yuchuan <[email protected]>
@Seven-Streams Seven-Streams force-pushed the fix/2025-11-07/tag_optim branch from 6d53e3a to 7c2cfd7 Compare November 16, 2025 07:16
@Seven-Streams Seven-Streams merged commit bdee539 into mlc-ai:main Nov 16, 2025
39 checks passed
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.

2 participants