Skip to content

Conversation

@lenzo-ka
Copy link
Contributor

@lenzo-ka lenzo-ka commented Oct 9, 2025

Summary

This PR increases FSG phonetic context support from 128 to 256 phones and simplifies the bitvector implementation for better maintainability. The motivation is that models with large numbers of phones otherwise will fail -- in particular, Chinese -- without recompilation.

Changes

  • Increase FSG_PNODE_CTXT_BVSZ from 4 to 8, supporting up to 256 phones (up from 128)
  • Replace compile-time conditional macros with generic implementation
  • Improve error message to show actual vs. maximum phone count

Rationale

The previous implementation required recompilation to support larger phonesets. This change:

  1. Supports larger phonesets: 256 phones covers all known phonetic inventories without custom builds
  2. Simplifies maintenance: Removes 20+ lines of conditional macro code in favor of generic function
  3. Negligible performance impact: Bitvector operations occur during word transitions, not in the acoustic scoring hot path (~0.002% overhead)

Memory Impact

  • Per fsg_pnode_t: +16 bytes (11% increase from ~144 to ~160 bytes)
  • Typical grammars: +160KB to 1.6MB for 10K-100K nodes
  • Acceptable tradeoff for runtime flexibility

Testing

  • Clean compilation with no errors
  • Existing unit tests pass
  • Backward compatible with existing models and grammars

Increase FSG_PNODE_CTXT_BVSZ from 4 to 8, supporting up to 256 phones
instead of 128. This covers all known phonesets without recompilation.

Replace compile-time macro variants with generic implementation to
simplify code maintenance. The performance impact is negligible as
these operations occur during word transitions, not acoustic scoring.

Memory impact: ~16 bytes per fsg_pnode (11% increase), typically
160KB-1.6MB additional for medium to large grammars.
@lenzo-ka lenzo-ka requested a review from dhdaines October 9, 2025 21:05
@lenzo-ka
Copy link
Contributor Author

lenzo-ka commented Oct 9, 2025

I looked at doing this dynamically at model load time based on the number of phones in the model, but the savings is like 160-240KB total.

@lenzo-ka
Copy link
Contributor Author

This still runs fine on existing models with <128 phones

@dhdaines
Copy link
Contributor

Yes, I agree that this is an okay change. For complex grammars there are much worse sources of memory inefficiency in the FSG search code than this!

@dhdaines dhdaines merged commit b86eb46 into main Oct 23, 2025
21 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants