Optimize DefaultDeterminizeStateTable with move semantics in CompactHashBiTable.#83
Open
copybara-service[bot] wants to merge 1 commit intomainfrom
Open
Optimize DefaultDeterminizeStateTable with move semantics in CompactHashBiTable.#83copybara-service[bot] wants to merge 1 commit intomainfrom
DefaultDeterminizeStateTable with move semantics in CompactHashBiTable.#83copybara-service[bot] wants to merge 1 commit intomainfrom
Conversation
649e0ff to
21555e9
Compare
DefaultDeterminizeStateTable with move semantics in CompactHashBiTable.
0ce9d42 to
a1094f1
Compare
DefaultDeterminizeStateTable with move semantics in CompactHashBiTable.a1094f1 to
662623f
Compare
DefaultDeterminizeStateTable with move semantics in CompactHashBiTable.
662623f to
d1e4178
Compare
…ctHashBiTable`. This change adds a perfect-forwarding `FindId` template (and a `const T&` overload to support initializer lists) to `CompactHashBiTable`, allowing entries to be efficiently copied or moved into the table. This is used in `DefaultDeterminizeStateTable` to avoid manual memory management and support storing `std::unique_ptr<StateTuple>`. In particular: - forcing a reference to `T` to allow implicit conversions (e.g. string literals to `std::string`). - addressing template deduction failures for initializer lists in `FindId` by providing a non-template `const T&` overload. Also added a new benchmark `BM_DeterminizeTransHighlyNonDeterministic` to stress test determinization with highly non-deterministic transducers generating long output strings. Impact (based on fair comparison with clean config and long strings): - Transducer determinization improved by ~9.3% (`BM_DeterminizeTrans`). - On-the-fly transducer determinization improved by ~17.6% (`BM_DeterminizeFstTrans`). - Acceptor determinization showed improvements of 2-3%. - The new highly non-deterministic test is performance-neutral. - Code cleanup: Removed manual memory management (raw pointers and manual delete) in `DefaultDeterminizeStateTable` to address an existing TODO. Also: - Fixed a potential use-after-free (null pointer dereference) in `FindState` where `tuple->subset` was accessed after `tuple` was moved into the state table. PiperOrigin-RevId: 900056191
d1e4178 to
e271850
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Optimize
DefaultDeterminizeStateTablewith move semantics inCompactHashBiTable.This change adds a perfect-forwarding
FindIdtemplate (and aconst T&overload to support initializer lists) toCompactHashBiTable, allowing entries to be efficiently copied or moved into the table. This is used inDefaultDeterminizeStateTableto avoid manual memory management and support storingstd::unique_ptr<StateTuple>.In particular:
Tto allow implicit conversions (e.g. string literals tostd::string).FindIdby providing a non-templateconst T&overload.Also added a new benchmark
BM_DeterminizeTransHighlyNonDeterministicto stress test determinization with highly non-deterministic transducers generating long output strings.Impact (based on fair comparison with clean config and long strings):
BM_DeterminizeTrans).BM_DeterminizeFstTrans).DefaultDeterminizeStateTableto address an existing TODO.Also:
FindStatewheretuple->subsetwas accessed aftertuplewas moved into the state table.