-
-
Notifications
You must be signed in to change notification settings - Fork 42
LT-22674: fix quadratic NFC offset-map build that made opening decomposed texts take 10+ seconds #1056
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
LT-22674: fix quadratic NFC offset-map build that made opening decomposed texts take 10+ seconds #1056
Changes from all commits
8ba0eba
09389b7
cbb4586
cc801ab
a666ea0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -25,6 +25,8 @@ namespace SIL.FieldWorks.Common.RootSites.RenderBenchmark | |
| public abstract class RenderBenchmarkTestsBase : RealDataTestsBase | ||
| { | ||
| protected const string DeterministicRenderFontFamily = "Segoe UI"; | ||
| // Second Latin font for writing-system-alternation scenarios. | ||
| protected const string SecondaryRenderFontFamily = "Times New Roman"; | ||
| // Pinned Arabic font (loaded privately by RenderTestAssemblySetup). Used for Arabic runs so | ||
| // they don't depend on the host's Segoe UI Arabic version / font fallback. | ||
| protected const string ArabicRenderFontFamily = "Scheherazade New"; | ||
|
|
@@ -320,6 +322,15 @@ protected void SetupScenarioData(string scenarioId) | |
| case "multi-ws": | ||
| CreateMultiWsScenario(); | ||
| break; | ||
| case "single-para-mixed-ws": | ||
| CreateSingleParaMixedWsScenario(); | ||
| break; | ||
| case "nfc-composable-diacritics": | ||
| CreateNfcComposableDiacriticsScenario(); | ||
| break; | ||
| case "multi-line-wrap-single-ws": | ||
| CreateMultiLineWrapSingleWsScenario(); | ||
| break; | ||
| case "lex-shallow": | ||
| CreateLexEntryScenario(depth: 2, breadth: 3); | ||
| break; | ||
|
|
@@ -538,6 +549,30 @@ private void CreateMultiWsScenario() | |
| AddMultiWsSections(book, 5, versesPerSection: 8, chapterStart: 1); | ||
| } | ||
|
|
||
| /// <summary>Single paragraph of many short sentences alternating two writing systems.</summary> | ||
| private void CreateSingleParaMixedWsScenario() | ||
| { | ||
| var book = CreateBook(19); // PSA | ||
| m_hvoRoot = book.Hvo; | ||
| AddSingleMixedWsParagraph(book, sentenceCount: 236); | ||
| } | ||
|
|
||
| /// <summary>Single wrapped paragraph of Latin words spelled with decomposed diacritics.</summary> | ||
| private void CreateNfcComposableDiacriticsScenario() | ||
| { | ||
| var book = CreateBook(15); // EZR | ||
| m_hvoRoot = book.Hvo; | ||
| AddNfcComposableDiacriticsParagraph(book, wordCount: 80); | ||
| } | ||
|
|
||
| /// <summary>Single long wrapped paragraph of unique sentences in one writing system.</summary> | ||
| private void CreateMultiLineWrapSingleWsScenario() | ||
| { | ||
| var book = CreateBook(17); // EST | ||
| m_hvoRoot = book.Hvo; | ||
| AddSingleWsProseParagraph(book, sentenceCount: 200); | ||
| } | ||
|
|
||
| #region Rich Data Factories | ||
|
|
||
| protected IScrBook CreateBook(int bookNum) | ||
|
|
@@ -927,6 +962,132 @@ protected void AddMultiWsSections(IScrBook book, int sectionCount, | |
| } | ||
| } | ||
|
|
||
| /// <summary>Single paragraph of alternating-writing-system sentences with decomposed diacritics.</summary> | ||
| protected void AddSingleMixedWsParagraph(IScrBook book, int sentenceCount) | ||
| { | ||
| var section = Cache.ServiceLocator.GetInstance<IScrSectionFactory>().Create(); | ||
| book.SectionsOS.Add(section); | ||
|
|
||
| var stTextFactory = Cache.ServiceLocator.GetInstance<IStTextFactory>(); | ||
|
|
||
| section.HeadingOA = stTextFactory.Create(); | ||
| var headingBldr = new StTxtParaBldr(Cache) { ParaStyleName = ScrStyleNames.SectionHead }; | ||
| headingBldr.AppendRun("Single Paragraph, Mixed Writing Systems", | ||
| StyleUtils.CharStyleTextProps(null, m_wsEng)); | ||
| headingBldr.CreateParagraph(section.HeadingOA); | ||
|
|
||
| string[] subjects = | ||
| { | ||
| "the élder", "the hèrder", "the sïnger", | ||
| "the teaçher", "the travêler", "the womãn" | ||
| }; | ||
| string[] predicates = | ||
| { | ||
| "spoke of the lóng rains", "walked to the fàr well", "named the sevën hills", | ||
| "counted the cattlé at dusk", "kept the ôld story", "asked for a blessĩng" | ||
| }; | ||
|
|
||
| section.ContentOA = stTextFactory.Create(); | ||
| var paraBldr = new StTxtParaBldr(Cache) { ParaStyleName = ScrStyleNames.NormalParagraph }; | ||
| paraBldr.AppendRun("1", StyleUtils.CharStyleTextProps(ScrStyleNames.ChapterNumber, m_wsEng)); | ||
|
|
||
| for (int i = 0; i < sentenceCount; i++) | ||
| { | ||
| string sentence = $"{subjects[i % subjects.Length]} {predicates[i % predicates.Length]} {i + 1}. "; | ||
| paraBldr.AppendRun(sentence, AlternatingFontRunProps(i % 2 == 0)); | ||
| } | ||
|
|
||
| paraBldr.CreateParagraph(section.ContentOA); | ||
| } | ||
|
|
||
| /// <summary>Run properties alternating writing system and font family together.</summary> | ||
| private ITsTextProps AlternatingFontRunProps(bool first) | ||
| { | ||
| return AlternatingFontRunProps(first, first); | ||
| } | ||
|
|
||
| /// <summary>Run properties combining an independently chosen writing system and font family.</summary> | ||
| private ITsTextProps AlternatingFontRunProps(bool useFirstWs, bool useFirstFont) | ||
| { | ||
| var bldr = TsStringUtils.MakePropsBldr(); | ||
| bldr.SetIntPropValues((int)FwTextPropType.ktptWs, (int)FwTextPropVar.ktpvDefault, | ||
| useFirstWs ? m_wsEng : m_wsFr); | ||
| bldr.SetStrPropValue((int)FwTextPropType.ktptFontFamily, | ||
| useFirstFont ? DeterministicRenderFontFamily : SecondaryRenderFontFamily); | ||
| return bldr.GetTextProps(); | ||
| } | ||
|
|
||
| /// <summary>Single wrapped paragraph of Latin words spelled with decomposed diacritics.</summary> | ||
| protected void AddNfcComposableDiacriticsParagraph(IScrBook book, int wordCount) | ||
| { | ||
| var section = Cache.ServiceLocator.GetInstance<IScrSectionFactory>().Create(); | ||
| book.SectionsOS.Add(section); | ||
|
|
||
| var stTextFactory = Cache.ServiceLocator.GetInstance<IStTextFactory>(); | ||
|
|
||
| section.HeadingOA = stTextFactory.Create(); | ||
| var headingBldr = new StTxtParaBldr(Cache) { ParaStyleName = ScrStyleNames.SectionHead }; | ||
| headingBldr.AppendRun("Decomposed Diacritics Microbenchmark", | ||
| StyleUtils.CharStyleTextProps(null, m_wsEng)); | ||
| headingBldr.CreateParagraph(section.HeadingOA); | ||
|
|
||
| string[] decomposedWords = | ||
| { | ||
| "café", "déjà", "noël", "français", "garçon", | ||
| "hôtel", "año", "créée", "élégant", "façade", | ||
| }; | ||
|
|
||
| section.ContentOA = stTextFactory.Create(); | ||
| var paraBldr = new StTxtParaBldr(Cache) { ParaStyleName = ScrStyleNames.NormalParagraph }; | ||
|
|
||
| for (int i = 0; i < wordCount; i++) | ||
| { | ||
| string word = decomposedWords[i % decomposedWords.Length]; | ||
| paraBldr.AppendRun($"The {word} recorded here is entry {i + 1}. ", | ||
| StyleUtils.CharStyleTextProps(null, m_wsEng)); | ||
| } | ||
|
|
||
| paraBldr.CreateParagraph(section.ContentOA); | ||
| } | ||
|
|
||
| /// <summary>Single long wrapped paragraph of unique sentences with decomposed diacritics, one writing system.</summary> | ||
| protected void AddSingleWsProseParagraph(IScrBook book, int sentenceCount) | ||
| { | ||
| var section = Cache.ServiceLocator.GetInstance<IScrSectionFactory>().Create(); | ||
| book.SectionsOS.Add(section); | ||
|
|
||
| var stTextFactory = Cache.ServiceLocator.GetInstance<IStTextFactory>(); | ||
|
|
||
| section.HeadingOA = stTextFactory.Create(); | ||
| var headingBldr = new StTxtParaBldr(Cache) { ParaStyleName = ScrStyleNames.SectionHead }; | ||
| headingBldr.AppendRun("Single Writing-System Line-Wrap Microbenchmark", | ||
| StyleUtils.CharStyleTextProps(null, m_wsEng)); | ||
| headingBldr.CreateParagraph(section.HeadingOA); | ||
|
|
||
| string[] subjects = | ||
| { | ||
| "the merçhant", "the masón", "the scribë", | ||
| "the shephèrd", "the weavêr", "the pottér" | ||
| }; | ||
| string[] predicates = | ||
| { | ||
| "measured the grain by the rivér", "repaired the eastèrn wall before dusk", | ||
| "copied the ledger onto fresh parchmënt", "counted the flock past the old gatê", | ||
| "dyed the cloth a deep saffrõn", "shaped the jar on the slow wheël" | ||
| }; | ||
|
|
||
| section.ContentOA = stTextFactory.Create(); | ||
| var paraBldr = new StTxtParaBldr(Cache) { ParaStyleName = ScrStyleNames.NormalParagraph }; | ||
|
|
||
| for (int i = 0; i < sentenceCount; i++) | ||
| { | ||
| string sentence = $"{subjects[i % subjects.Length]} {predicates[i % predicates.Length]} on day {i + 1}. "; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. same here |
||
| paraBldr.AppendRun(sentence, StyleUtils.CharStyleTextProps(null, m_wsEng)); | ||
| } | ||
|
|
||
| paraBldr.CreateParagraph(section.ContentOA); | ||
| } | ||
|
|
||
| #endregion | ||
|
|
||
| #region Lex Entry Scenario Data | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -29,8 +29,6 @@ DEFINE_THIS_FILE | |
| //:>******************************************************************************************** | ||
| //:> Forward declarations | ||
| //:>******************************************************************************************** | ||
| static void BuildNfcOffsetMaps(const StrUni & stuOrig, Vector<int> & vichOrigToNfc, | ||
| Vector<int> & vichNfcToOrig); | ||
| static void ApplyShapeRunCacheEntry(ShapeRunEntry & entry, UniscribeRunInfo & uri); | ||
|
|
||
| //:>******************************************************************************************** | ||
|
|
@@ -1702,30 +1700,6 @@ int UniscribeSegment::OffsetToOrig(int ich, int ichBase, IVwTextSource * pts, bo | |
| return OffsetToOrig(ich, ichBase, pts, fTextIsNfc); | ||
| } | ||
|
|
||
| static void BuildNfcOffsetMaps(const StrUni & stuOrig, Vector<int> & vichOrigToNfc, | ||
| Vector<int> & vichNfcToOrig) | ||
| { | ||
| int cchOrig = stuOrig.Length(); | ||
| vichOrigToNfc.Resize(cchOrig + 1); | ||
| vichOrigToNfc[0] = 0; | ||
| for (int ich = 1; ich <= cchOrig; ++ich) | ||
| { | ||
| StrUni stuPrefix(stuOrig.Chars(), ich); | ||
| StrUtil::NormalizeStrUni(stuPrefix, UNORM_NFC); | ||
| vichOrigToNfc[ich] = stuPrefix.Length(); | ||
| } | ||
|
|
||
| int cchNfc = vichOrigToNfc[cchOrig]; | ||
| vichNfcToOrig.Resize(cchNfc + 1); | ||
| int ichOrig = 0; | ||
| for (int ichNfc = 0; ichNfc <= cchNfc; ++ichNfc) | ||
| { | ||
| while (ichOrig + 1 <= cchOrig && vichOrigToNfc[ichOrig + 1] <= ichNfc) | ||
| ++ichOrig; | ||
| vichNfcToOrig[ichNfc] = ichOrig; | ||
| } | ||
| } | ||
|
|
||
| static void ApplyShapeRunCacheEntry(ShapeRunEntry & entry, UniscribeRunInfo & uri) | ||
| { | ||
| if (uri.CGlyphMax() < entry.m_cglyph) | ||
|
|
@@ -3135,6 +3109,8 @@ int UniscribeSegment::CallScriptItemize(OLECHAR * prgchDefBuf, int cchBuf, | |
|
|
||
| Vector<int> vichOrigToNfc; | ||
| Vector<int> vichNfcToOrig; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These two vectors seem to be set to |
||
| // Only text that normalization leaves unchanged is eligible for the analysis cache. | ||
| bool fTextEligibleForCache = true; | ||
|
|
||
| #ifdef UNISCRIBE_NFC | ||
| if (cch) | ||
|
|
@@ -3154,8 +3130,7 @@ int UniscribeSegment::CallScriptItemize(OLECHAR * prgchDefBuf, int cchBuf, | |
| bool fComputedTextIsNfc = (stu == stuOrig); | ||
| if (pfTextIsNfc) | ||
| *pfTextIsNfc = fComputedTextIsNfc; | ||
| if (!fComputedTextIsNfc && pLayoutPassCache) | ||
| BuildNfcOffsetMaps(stuOrig, vichOrigToNfc, vichNfcToOrig); | ||
| fTextEligibleForCache = fComputedTextIsNfc; | ||
| if (cch > cchBuf) | ||
| { | ||
| cchBuf = cch; | ||
|
|
@@ -3317,10 +3292,14 @@ typedef struct tag_SCRIPT_STATE { | |
| if (pLayoutPassCache) | ||
| { | ||
| pLayoutPassCache->AnalysisCache().AddComputeMs(::GetTickCount() - dwStartMs); | ||
| TextAnalysisEntry * pStoredAnalysis = pLayoutPassCache->AnalysisCache().Store(pts, ichMin, | ||
| cchOrig, ws, fWsRtl, *pprgchBuf, cch, pfTextIsNfc ? *pfTextIsNfc : true, | ||
| g_vscri.Begin(), citem, vichOrigToNfc.Size() ? &vichOrigToNfc : NULL, | ||
| vichNfcToOrig.Size() ? &vichNfcToOrig : NULL); | ||
| TextAnalysisEntry * pStoredAnalysis = NULL; | ||
| if (fTextEligibleForCache) | ||
| { | ||
| pStoredAnalysis = pLayoutPassCache->AnalysisCache().Store(pts, ichMin, | ||
| cchOrig, ws, fWsRtl, *pprgchBuf, cch, true, | ||
| g_vscri.Begin(), citem, vichOrigToNfc.Size() ? &vichOrigToNfc : NULL, | ||
| vichNfcToOrig.Size() ? &vichNfcToOrig : NULL); | ||
| } | ||
| if (ppAnalysis) | ||
| *ppAnalysis = pStoredAnalysis; | ||
| } | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't see why we need separate subjects and predicates if they're always combined into a single run.
Slick alternating run generator, though! 👍🏻