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.

Sign up for GitHub

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

Jump to bottom

fix: 🐛 #2581 修复 scheduler 任务调度的问题 (#2581) #2582

Merged
merged 2 commits into from
Aug 29, 2022
Merged

fix: 🐛 #2581 修复 scheduler 任务调度的问题 (#2581) #2582

merged 2 commits into from
Aug 29, 2022

Conversation

AricZhu
Copy link

@AricZhu AricZhu commented Aug 26, 2022 •
edited
Loading

Description

Motivation and Context

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Enhancement (changes that improvement of current feature or performance)
  • Refactoring (changes that neither fixes a bug nor adds a feature)
  • Test Case (changes that add missing tests or correct existing tests)
  • Code style optimization (changes that do not affect the meaning of the code)
  • Docs (changes that only update documentation)
  • Chore (changes that don't modify src or test files)

Self Check before Merge

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@x6-bot x6-bot bot added pkg:x6 labels Aug 26, 2022
@x6-bot
Copy link
Contributor

x6-bot bot commented Aug 26, 2022

👋 @AricZhu

💖 Thanks for opening this pull request! 💖

Please follow the contributing guidelines. And we use semantic commit messages to streamline the release process.

Examples of commit messages with semantic prefixes:

  • fix: don't overwrite prevent_default if default wasn't prevented
  • feat: add graph.scale() method
  • docs: graph.getShortestPath is now available

Things that will help get your PR across the finish line:

  • Follow the TypeScript, JavaScript, CSS and React coding style.
  • Run npm run lint locally to catch formatting errors earlier.
  • Document any user-facing changes you've made.
  • Include tests when adding/changing behavior.
  • Include screenshots and animated GIFs whenever possible.

We get a lot of pull requests on this repo, so please be patient and we will get back to you as soon as we can.

@x6-bot x6-bot bot added PR: unreviewed PR does not have any reviews. needs-more-info labels Aug 26, 2022
@x6-bot
Copy link
Contributor

x6-bot bot commented Aug 26, 2022

@AricZhu Please provide us with more info about this pull request.

@x6-bot x6-bot bot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Aug 26, 2022
@AricZhu AricZhu mentioned this pull request Aug 26, 2022
Closed
@codecov
Copy link

codecov bot commented Aug 26, 2022 •
edited
Loading

Codecov Report

Merging #2582 (78afd09) into master (7405eba) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #2582   +/-   ##
=======================================
  Coverage   40.76%   40.76%           
=======================================
  Files         542      542           
  Lines       32395    32395           
  Branches     6608     6608           
=======================================
  Hits        13207    13207           
  Misses      18396    18396           
  Partials      792      792
Flag Coverage Δ
x6 26.87% <100.00%> (ø)
x6-geometry 63.74% <ø> (ø)
x6-vector 94.23% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
packages/x6/src/util/scheduler/index.ts 44.11% <100.00%> (ø)

NewByVector
NewByVector requested changes Aug 28, 2022
View reviewed changes
packages/x6/src/util/scheduler/index.ts
@@ -17,7 +17,7 @@ export namespace Scheduler {
const schedule = (cb: FlushTaskFn) => unit.push(cb) === 1 && postMessage()

const postMessage = (() => {
const cb = () => unit.splice(0, unit.length).forEach((c) => c())
const cb = () => unit.splice(0, unit.length)[0]?.()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

直接使用 unit.splice(0, 1).forEach((c) => c()) 比较好。

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这里不清空 unit 队列的话会有 bug，会导致任务超时被挂起后，后续就再也不会被触发了（schedule 中由于 unit 没有被清空，所以永远满足不了 length 为 1 的条件，后续的 postMessage 也就不会被调用）

@x6-bot x6-bot bot added PR: reviewed-changes-requested PR has reviewed and got Change request event. and removed PR: unreviewed PR does not have any reviews. labels Aug 28, 2022
@x6-bot x6-bot bot added PR: reviewed-approved PR has reviewed and got Approve from everyone. and removed PR: reviewed-changes-requested PR has reviewed and got Change request event. labels Aug 29, 2022
@NewByVector NewByVector merged commit 502ceb1 into antvis:master Aug 29, 2022
@x6-bot x6-bot bot removed the PR: reviewed-approved PR has reviewed and got Approve from everyone. label Aug 29, 2022
@x6-bot
Copy link
Contributor

x6-bot bot commented Aug 29, 2022

👋 @AricZhu

Congrats on merging your first pull request! 🎉🎉🎉

@x6-bot x6-bot bot added the PR: merged PR has merged. label Aug 29, 2022
@NewByVector NewByVector linked an issue Aug 30, 2022 that may be closed by this pull request
scheduler调度的bug #2581
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@NewByVector NewByVector NewByVector approved these changes

Assignees
No one assigned
Labels
PR: merged PR has merged. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

scheduler调度的bug
2 participants
@AricZhu @NewByVector