diff --git a/CHANGELOG.md b/CHANGELOG.md index 634f1734..dca58ea7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,9 +10,7 @@ All notable changes to this project are documented in this file. This file follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/) conventions. Version numbers follow [Semantic Versioning](https://semver.org/). -## [Unreleased] - -## [2.0.0] - 2026-04-01 +## [2.0.0] - 2026-04-03 ### Added @@ -103,18 +101,24 @@ conventions. Version numbers follow [Semantic Versioning](https://semver.org/). Use tokenizer class instances instead. - Custom four-byte string implementation removed. Use `CharString` instead. +### Fixed + +- `src/tokenizer/newmm.rs`: Resolved exponential BFS path explosion in + `bfs_paths_graph` (#101). + - Mirrors the fix in PyThaiNLP/pythainlp#1319. + ### Migration guide #### Python ```python -# Before (v1.x / early v2.0) +# Before (v1.x) from nlpo3 import load_dict, segment, DeepcutTokenizer, segment_deepcut load_dict("path/to/dict.txt", "mydict") tokens = segment("สวัสดีครับ", "mydict") tokens = segment_deepcut("สวัสดีครับ") -# After (v2.0) +# Now (v2.0) from nlpo3 import NewmmTokenizer, NewmmFstTokenizer, DeepcutTokenizer tok = NewmmTokenizer("path/to/dict.txt") tokens = tok.segment("สวัสดีครับ") @@ -124,40 +128,16 @@ tokens = DeepcutTokenizer().segment("สวัสดีครับ") #### Node.js / TypeScript ```typescript -// Before (v1.x / early v2.0) +// Before (v1.x) loadDict("path/to/dict.txt", "mydict"); const tokens = segment("สวัสดีครับ", "mydict", false, false); -// After (v2.0) +// Now (v2.0) import { NewmmTokenizer } from "nlpo3-nodejs"; const tok = new NewmmTokenizer("path/to/dict.txt"); const tokens = tok.segment("สวัสดีครับ"); ``` -#### CLI - -```bash -# Before -echo "สวัสดีครับ" | nlpo3 segment --dict-path /path/to/dict.txt - -# After (newmm is still default, dict-path still works) -echo "สวัสดีครับ" | nlpo3 segment --dict-path /path/to/dict.txt - -# New: choose tokenizer -echo "สวัสดีครับ" | nlpo3 segment -t nf -echo "สวัสดีครับ" | nlpo3 segment -t deepcut -``` - -### Performance (from `BENCHMARK_RESULTS.md`) - -All three tokenizers use the same `Tokenizer` trait and are interchangeable: - -| Tokenizer | short (28 ch) | long (937 ch) | Dict memory | -| --------- | ------------- | ------------- | ----------- | -| `NewmmTokenizer` | **2.63 µs** | **165 µs** | ~43 MB | -| `NewmmFstTokenizer` | 29.5 µs | 2 225 µs | **~0.85 MB** | -| `DeepcutTokenizer` | - | - | ~3.9 MB model | - ## [1.4.0] - 2024-11-09 ### Changed diff --git a/src/tokenizer/newmm.rs b/src/tokenizer/newmm.rs index 405ac033..e3dc5cbb 100644 --- a/src/tokenizer/newmm.rs +++ b/src/tokenizer/newmm.rs @@ -278,6 +278,12 @@ impl NewmmTokenizer { ) -> AnyResult> { current_queue.clear(); + // The visited set ensures each vertex is enqueued at most once. + // This avoids the exponential blow-up from revisiting the same nodes. + let mut visited: HashSet = + HashSet::with_capacity_and_hasher(goal - start, Default::default()); + visited.insert(start); + let mut init_path: Vec = Vec::with_capacity(goal - start); init_path.push(start); current_queue.push_back((start, init_path)); @@ -285,15 +291,16 @@ impl NewmmTokenizer { while let Some((vertex, path)) = current_queue.pop_front() { if let Some(idk) = graph.get(&vertex) { for &position in idk { - if position != goal { - let mut appended_path = path.clone(); - appended_path.push(position); - current_queue.push_back((position, appended_path)); - } else { + if position == goal { let mut appended_path = path; appended_path.push(position); return Ok(appended_path); - }; + } else if !visited.contains(&position) { + visited.insert(position); + let mut appended_path = path.clone(); + appended_path.push(position); + current_queue.push_back((position, appended_path)); + } } }; } @@ -354,7 +361,8 @@ impl NewmmTokenizer { first_position_list, &mut reused_queue, )?; - graph_size = 0; // reset graph + graph_size = 0; + graph.clear(); for &position in group_of_end_position_candidate.iter().skip(1) { let token = text.substring_as_str(end_position, position); @@ -421,11 +429,8 @@ impl NewmmTokenizer { } } - graph - .entry(begin_position) - .or_insert_with(|| Vec::with_capacity(10)) - .push(end_position); - graph_size += 1; + graph_size = 0; + graph.clear(); let token = text.substring_as_str(begin_position, end_position); result_str.push(token); position_list.push(end_position); diff --git a/tests/test_tokenizer.rs b/tests/test_tokenizer.rs index 17eb1afe..d1417c15 100644 --- a/tests/test_tokenizer.rs +++ b/tests/test_tokenizer.rs @@ -366,3 +366,55 @@ fn test_tokenizer_trait_switchable() { fst.segment_to_string(text, false, false), ); } + +/// Regression test for BFS path explosion in `bfs_paths_graph`. +/// +/// Without the visited-set fix, highly ambiguous tokenization input causes +/// the BFS queue to grow exponentially (O(2^n)), making this test run for +/// many seconds or minutes. With the fix, it must complete in well under +/// one second. +#[test] +fn test_newmm_ambiguous_performance() { + use std::time::{Duration, Instant}; + + // Build a dictionary with heavily overlapping words (lengths 1–3) using + // a small set of standalone Thai consonants. Each consonant is its own + // Thai Character Cluster (TCC), so every character boundary is a valid + // split point, maximizing the number of ambiguous paths. + let chars = ["ก", "ข", "ค", "ง", "จ"]; + let mut words: Vec = Vec::new(); + for &c in &chars { + words.push(c.to_string()); + } + for &c1 in &chars { + for &c2 in &chars { + words.push(format!("{c1}{c2}")); + } + } + for &c1 in &chars { + for &c2 in &chars { + for &c3 in &chars { + words.push(format!("{c1}{c2}{c3}")); + } + } + } + + let tokenizer = NewmmTokenizer::from_word_list(words); + + // 50 repetitions of a five-consonant sequence = 250 characters. + // Without the fix this input causes exponential BFS expansion. + let text = "กขคงจ".repeat(50); + + let start = Instant::now(); + let result = tokenizer.segment_to_string(&text, false, false); + let elapsed = start.elapsed(); + + assert!( + !result.is_empty(), + "tokenizer must produce at least one token" + ); + assert!( + elapsed < Duration::from_secs(1), + "tokenization took {elapsed:?}; BFS path explosion may still be present" + ); +}