Optimize performance of LG evaluation #3327

Danieladu · 2020-02-03T07:40:24Z

Today, every time a free LG text like "@{welcome}" is evaluated by creating a new "anonymous template" to wrap this free text, and create a temporary LG file with the new content = original content + new anonymous template, for evaluation.

This is not performance friendly. In this pr, we just parse the additional part, so that there is no need to reparse the original content.

coveralls · 2020-02-03T07:55:28Z

Pull Request Test Coverage Report for Build 103269

0 of 0 changed or added relevant lines in 0 files are covered.
96 unchanged lines in 5 files lost coverage.
Overall coverage decreased (-3.6%) to 78.027%

Files with Coverage Reduction	New Missed Lines	%
/libraries/Microsoft.Bot.Builder.LanguageGeneration/Range.cs	4	68.75%
/libraries/Microsoft.Bot.Builder.LanguageGeneration/CustomizedMemory.cs	5	69.7%
/libraries/Microsoft.Bot.Builder.LanguageGeneration/LGFile.cs	14	85.56%
/libraries/Microsoft.Bot.Builder.LanguageGeneration/Evaluator.cs	29	91.74%
/libraries/Microsoft.Bot.Builder.LanguageGeneration/Expander.cs	44	84.24%

Totals
Change from base Build 103006:	-3.6%
Covered Lines:	21157
Relevant Lines:	27115

💛 - Coveralls

libraries/Microsoft.Bot.Expressions.Properties/Extensions.cs

fuselabs · 2020-02-03T07:58:48Z

✔️ No Binary Compatibility issues for Microsoft.Bot.Builder.dll compared against version 4.6.3
✔️ No Binary Compatibility issues for Microsoft.Bot.Builder.AI.Luis.dll compared against version 4.6.3
✔️ No Binary Compatibility issues for Microsoft.Bot.Builder.AI.QnA.dll compared against version 4.6.3
✔️ No Binary Compatibility issues for Microsoft.Bot.Builder.ApplicationInsights.dll compared against version 4.6.3
✔️ No Binary Compatibility issues for Microsoft.Bot.Builder.Azure.dll compared against version 4.6.3
✔️ No Binary Compatibility issues for Microsoft.Bot.Builder.Dialogs.dll compared against version 4.6.3
✔️ No Binary Compatibility issues for Microsoft.Bot.Builder.Integration.ApplicationInsights.Core.dll compared against version 4.6.3
✔️ No Binary Compatibility issues for Microsoft.Bot.Builder.Integration.AspNet.Core.dll compared against version 4.6.3
✔️ No Binary Compatibility issues for Microsoft.Bot.Builder.TemplateManager.dll compared against version 4.6.3
✔️ No Binary Compatibility issues for Microsoft.Bot.Builder.Testing.dll compared against version 4.6.3
✔️ No Binary Compatibility issues for Microsoft.Bot.Configuration.dll compared against version 4.6.3
✔️ No Binary Compatibility issues for Microsoft.Bot.Connector.dll compared against version 4.6.3
✔️ No Binary Compatibility issues for Microsoft.Bot.Schema.dll compared against version 4.6.3
✔️ No Binary Compatibility issues for Microsoft.Bot.Streaming.dll compared against version 4.6.3

libraries/Microsoft.Bot.Builder.LanguageGeneration/CustomizedMemory.cs

boydc2014 · 2020-02-03T08:04:02Z

Looks good, one more thing i have is can we measure\profile this change, see how much we improved.

Some suggestions to profile

compare the time used of finishing all tests, make sure you run it multiple times to warm up
compare the time used of finishing one specific time
compare the time used of one single Evaluate method, perhaps in debug mode in VS and check cpu time?
ChrisMc also mentioned a way to profile in the email, please try that if possible.

fuselabs · 2020-02-03T09:43:31Z

✔️ No Binary Compatibility issues for Microsoft.Bot.Builder.dll compared against version 4.6.3
✔️ No Binary Compatibility issues for Microsoft.Bot.Builder.AI.Luis.dll compared against version 4.6.3
✔️ No Binary Compatibility issues for Microsoft.Bot.Builder.AI.QnA.dll compared against version 4.6.3
✔️ No Binary Compatibility issues for Microsoft.Bot.Builder.ApplicationInsights.dll compared against version 4.6.3
✔️ No Binary Compatibility issues for Microsoft.Bot.Builder.Azure.dll compared against version 4.6.3
✔️ No Binary Compatibility issues for Microsoft.Bot.Builder.Dialogs.dll compared against version 4.6.3
✔️ No Binary Compatibility issues for Microsoft.Bot.Builder.Integration.ApplicationInsights.Core.dll compared against version 4.6.3
✔️ No Binary Compatibility issues for Microsoft.Bot.Builder.Integration.AspNet.Core.dll compared against version 4.6.3
✔️ No Binary Compatibility issues for Microsoft.Bot.Builder.TemplateManager.dll compared against version 4.6.3
✔️ No Binary Compatibility issues for Microsoft.Bot.Builder.Testing.dll compared against version 4.6.3
✔️ No Binary Compatibility issues for Microsoft.Bot.Configuration.dll compared against version 4.6.3
✔️ No Binary Compatibility issues for Microsoft.Bot.Connector.dll compared against version 4.6.3
✔️ No Binary Compatibility issues for Microsoft.Bot.Schema.dll compared against version 4.6.3
✔️ No Binary Compatibility issues for Microsoft.Bot.Streaming.dll compared against version 4.6.3

Danieladu · 2020-02-03T11:51:01Z

Looks good, one more thing i have is can we measure\profile this change, see how much we improved.

Some suggestions to profile

compare the time used of finishing all tests, make sure you run it multiple times to warm up

compare the time used of finishing one specific time

compare the time used of one single Evaluate method, perhaps in debug mode in VS and check cpu time?

ChrisMc also mentioned a way to profile in the email, please try that if possible.

Before optimization, the time used of finishing all adaptive tests is 45s,
After optimization, the time is 15s.
Before optimization, the time used of Evaluate with 100 iterations is 11s,
After optimization, the time is 5s.
for the solution provided by ChrisMc, I always get a 401 error when calling luis's api. I will try it later.

boydc2014 · 2020-02-03T13:02:16Z

Looks good, one more thing i have is can we measure\profile this change, see how much we improved.
Some suggestions to profile

compare the time used of finishing all tests, make sure you run it multiple times to warm up

compare the time used of finishing one specific time

compare the time used of one single Evaluate method, perhaps in debug mode in VS and check cpu time?

ChrisMc also mentioned a way to profile in the email, please try that if possible.

Before optimization, the time used of finishing all adaptive tests is 45s,
After optimization, the time is 15s.

Before optimization, the time used of Evaluate with 100 iterations is 11s,
After optimization, the time is 5s.

for the solution provided by ChrisMc, I always get a 401 error when calling luis's api. I will try it later.

Looks good. One more thing is, it would be great if we put more context in 'Description' of this PR, for example,

if it's a bug fix, you might want to list root cause + how it's fixed + potential impact if any
if it's a feature, you might want to list the description of the feature, how the users will benefit or be impacted.
if it's a perform tuning, (you can figure it out).

Anyhow, more context help both you and reviewers to have a better context

chrimc62 · 2020-02-03T22:28:53Z

libraries/Microsoft.Bot.Expressions.Properties/Extensions.cs


            var newLgFile = LGParser.ParseText(newContent, lgFile.Id, lgFile.ImportResolver);
-            return newLgFile.EvaluateTemplate(fakeTemplateId, scope);
+
+            var allTemplates = lgFile.AllTemplates.Union(newLgFile.AllTemplates).ToList();


couldn't you precompute this and only redo when new templates are added?

this newLgFile is computed from the parameter inlineStr, so we can't pre-compute it. What we can do is to cache this string, to avoid re-compute, but based on the profiling result, not much improvement here, because the inlineStr is very small. So this is good enough for now.

Optimize performance of LG evaluation

0a0da9a

Danieladu requested review from boydc2014, cosmicshuai and feich-ms February 3, 2020 07:40

boydc2014 reviewed Feb 3, 2020

View reviewed changes

libraries/Microsoft.Bot.Expressions.Properties/Extensions.cs Outdated Show resolved Hide resolved

boydc2014 reviewed Feb 3, 2020

View reviewed changes

libraries/Microsoft.Bot.Builder.LanguageGeneration/CustomizedMemory.cs Outdated Show resolved Hide resolved

wrap imemory

dc72255

Danieladu requested review from boydc2014 and chrimc62 February 3, 2020 11:51

chrimc62 approved these changes Feb 3, 2020

View reviewed changes

Danieladu merged commit 4df966c into master Feb 4, 2020

Danieladu deleted the hond/optimize-lg branch February 4, 2020 02:38

This was referenced Feb 4, 2020

[PORT] Optimize performance of LG evaluation microsoft/botbuilder-python#697

Closed

[PORT] Optimize performance of LG evaluation microsoft/botbuilder-java#256

Closed

[PORT] Optimize performance of LG evaluation microsoft/botbuilder-js#1660

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Optimize performance of LG evaluation #3327

Optimize performance of LG evaluation #3327

Danieladu commented Feb 3, 2020 •

edited

coveralls commented Feb 3, 2020 •

edited

fuselabs commented Feb 3, 2020

boydc2014 commented Feb 3, 2020

fuselabs commented Feb 3, 2020

Danieladu commented Feb 3, 2020 •

edited

boydc2014 commented Feb 3, 2020

chrimc62 Feb 3, 2020

boydc2014 Feb 4, 2020

Optimize performance of LG evaluation #3327

Optimize performance of LG evaluation #3327

Conversation

Danieladu commented Feb 3, 2020 • edited

coveralls commented Feb 3, 2020 • edited

Pull Request Test Coverage Report for Build 103269

💛 - Coveralls

fuselabs commented Feb 3, 2020

boydc2014 commented Feb 3, 2020

fuselabs commented Feb 3, 2020

Danieladu commented Feb 3, 2020 • edited

boydc2014 commented Feb 3, 2020

chrimc62 Feb 3, 2020

Choose a reason for hiding this comment

boydc2014 Feb 4, 2020

Choose a reason for hiding this comment

Danieladu commented Feb 3, 2020 •

edited

coveralls commented Feb 3, 2020 •

edited

Danieladu commented Feb 3, 2020 •

edited