Skip to content

refactor: share rule instances to speed up FST construction#333

Merged
pengzhendong merged 1 commit into
masterfrom
refactor/share-rule-instances
Jun 9, 2026
Merged

refactor: share rule instances to speed up FST construction#333
pengzhendong merged 1 commit into
masterfrom
refactor/share-rule-instances

Conversation

@pengzhendong

@pengzhendong pengzhendong commented Jun 9, 2026

Copy link
Copy Markdown
Member

Summary

  • Rule classes now accept optional dependency params (e.g. cardinal=None), reusing injected instances instead of creating new ones internally
  • Normalizer/InverseNormalizer build all rules once in topological order via build_tagger_and_verbalizer(), sharing instances across tagger and verbalizer construction
  • Processor base class falls back to separate build_tagger() + build_verbalizer() for backward compatibility

Before: Cardinal() was instantiated 60 times per English Normalizer build.
After: Cardinal() is instantiated once.

Performance

Metric Before After Speedup
English Normalizer build 71s 17s 4x
Full test suite 196s 88s 2.2x

Test plan

  • All 1393 unit tests pass locally
  • CI passes on PR

Each rule class now accepts optional dependency params (e.g. cardinal=None)
and reuses injected instances instead of creating its own. Normalizer and
InverseNormalizer build all rules once in topological order via a new
build_tagger_and_verbalizer() method, sharing instances across tagger and
verbalizer construction.

Before: Cardinal was instantiated 60 times per English Normalizer build.
After: Cardinal is instantiated once.

Performance improvement:
- English Normalizer build: 71s -> 17s (4x faster)
- Full test suite: 196s -> 88s (2.2x faster)
@pengzhendong pengzhendong changed the title refactor: share rule instances to eliminate redundant FST construction refactor: share rule instances to speed up FST construction Jun 9, 2026
@pengzhendong pengzhendong merged commit ab8a034 into master Jun 9, 2026
1 check passed
@pengzhendong pengzhendong deleted the refactor/share-rule-instances branch June 9, 2026 07:19
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