Skip to content
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

fix: Move persistence layer's delta computation into worker #5563

Merged
merged 11 commits into from
Jan 25, 2021

Conversation

lei9444
Copy link
Contributor

@lei9444 lei9444 commented Jan 20, 2021

Description

The persistence layer will check files' change after the state changed. When we use a big bot such as the calendarSkill, the check function will block the main thread about 90ms every notify.

fix
Move the complex computation to worker

Task Item

closes #5561

Screenshots

before:
image

after:
image

@coveralls
Copy link

coveralls commented Jan 20, 2021

Coverage Status

Coverage decreased (-0.04%) to 55.052% when pulling 00351b5 on lei9444:worker into d1269eb on microsoft:main.

@boydc2014
Copy link
Contributor

boydc2014 commented Jan 20, 2021

I can see the perf gain here, just one more question, do we know why after moving to worker, we still take about 10ms?

@lei9444
Copy link
Contributor Author

lei9444 commented Jan 20, 2021

I can see the perf gain here, just one more question, do we know why after moving to worker, we still take about 10ms?

The worker.postMessage will take some time here, postMessage its own will do some clone algorithm. I tried to use JSON.stringify our data, but looks like the JSON.stringify will take more time.

@srinaath
Copy link
Contributor

This is awesome work @lei9444 ! Moving it a worker is so much better.

@srinaath srinaath self-assigned this Jan 22, 2021
srinaath
srinaath previously approved these changes Jan 24, 2021
@lei9444 lei9444 merged commit 221dcec into microsoft:main Jan 25, 2021
alanlong9278 added a commit that referenced this pull request Jan 25, 2021
* main:
  Update numberinput.dialog (#5575)
  fix hover display incorrect returntype (#5588)
  fix: Move persistence layer's delta computation into worker (#5563)
  fix: electron update error (#5573)
  fix: showing correct error message in local publish (#5509)
  feat: change source of packages from local feed to live npm/nuget feed (#5516)
  set max http header size to fix 431 (#5521)
  Updating to daily runtime for R12 development (#5529)
  fix: correctly generate l10n files when using zsh (#5555)
  chore: deprecate feature request issue template (#5378)
  fix: designPage navigation to settings Page url error (#5546)
  fix: luis\qna key missing in skill bot (#5545)
  delete trigger by projectId passed from projectTree (#5542)
@lei9444 lei9444 deleted the worker branch February 1, 2021 02:06
@boydc2014 boydc2014 mentioned this pull request Feb 2, 2021
lei9444 added a commit to lei9444/BotFramework-Composer-1 that referenced this pull request Jun 15, 2021
…t#5563)

* fix: Move persistence layer's delta computation into worker

* fix unit testsa

* fix tests

* fix tests

* add test coverage

* update naming

Co-authored-by: Dong Lei <donglei@microsoft.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Move persistence layer's delta computation into worker
4 participants