Open
Labels
area-TokenizersText tokenisation and tokenizer APIsText tokenisation and tokenizer APIstenet-performancePerformance related issuePerformance related issue
Description
The current implementation of Word.MergeAll uses Vec<T>.Remove() to remove symbols from the list:
machinelearning/src/Microsoft.ML.Tokenizers/Model/Word.cs
Lines 187 to 194 in 81310b2
| // Filter out the removed symbols | |
| for (int i = _symbols.Count - 1; i >= 0; i--) | |
| { | |
| if (_symbols[i].Len == 0) | |
| { | |
| _symbols.Remove(i); | |
| } | |
| } |
Based on data I found, it seems it's common that around 75 % of symbols are removed here (though the data conflates multiple steps, so it could be not fully relevant). This makes the method quadratic, likely significantly affecting its performance.
We should consider improving that. (This won't make the method linear, since the use of PriorityQueue<T> leads to at least O(n log n).)
Metadata
Metadata
Assignees
Labels
area-TokenizersText tokenisation and tokenizer APIsText tokenisation and tokenizer APIstenet-performancePerformance related issuePerformance related issue
Activity