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

feat: support worker_threads #101

Merged
merged 4 commits into from
Nov 2, 2022
Merged

feat: support worker_threads #101

merged 4 commits into from
Nov 2, 2022

Conversation

hyj1991
Copy link
Member

@hyj1991 hyj1991 commented Oct 30, 2022

Checklist
  • npm test passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines
Affected core subsystem(s)

Deploy

Description of change

相比 #100 的一些改动:

  • 优化了下 Start Mode 的设计和目录结构
  • 最低版本限制到 node-v14.x,去除 worker_threads 存在判断
  • 部分已知 Bug 修复

另外,node-v18.x 因为底层 API 输出改动,旧的模块测试会大量挂掉,改动评估比较大,故另开 PR 修复,因此本 PR 的 CI 只验证 14 和 16 下的兼容性。

Progress
  • impl agent with worker_threads
  • impl app worker with worker_threads
  • tests and/or benchmarks are included
Reference

@hyj1991 hyj1991 marked this pull request as draft October 30, 2022 03:37
@hyj1991
Copy link
Member Author

hyj1991 commented Oct 30, 2022

对于 agentapp worker 的改造都比较大,会 rebase 成两个 commit,可以先看下 Draft 中 agent 部分局部重构后的逻辑是否还有需要优化的。

app worker 稍后补上,会去掉 cluster 的多端口 send 实现,直接启动 http(s) server,多 worker 模型下依靠 detectPort 自动累加分配,缺点是这个方案就需要固定搭配 nginx 做反向代理了。

cc @fengmk2 @atian25

@codecov
Copy link

codecov bot commented Oct 30, 2022

Codecov Report

Base: 98.96% // Head: 93.51% // Decreases project coverage by -5.44% ⚠️

Coverage data is based on head (fbbcdf3) compared to base (efa8441).
Patch coverage: 86.16% of modified lines in pull request are covered.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #101      +/-   ##
==========================================
- Coverage   98.96%   93.51%   -5.45%     
==========================================
  Files           8       12       +4     
  Lines         580      787     +207     
  Branches      105      119      +14     
==========================================
+ Hits          574      736     +162     
- Misses          6       51      +45     
Impacted Files Coverage Δ
lib/utils/options.js 97.50% <ø> (ø)
lib/utils/mode/impl/worker_threads/agent.js 67.34% <67.34%> (ø)
lib/utils/mode/impl/worker_threads/app.js 68.67% <68.67%> (ø)
lib/agent_worker.js 96.29% <83.33%> (-3.71%) ⬇️
lib/app_worker.js 98.50% <87.50%> (-1.50%) ⬇️
lib/utils/mode/impl/process/agent.js 92.85% <92.85%> (ø)
lib/master.js 100.00% <100.00%> (+1.28%) ⬆️
lib/utils/manager.js 97.61% <100.00%> (+0.25%) ⬆️
lib/utils/messenger.js 98.33% <100.00%> (-1.67%) ⬇️
lib/utils/mode/impl/process/app.js 100.00% <100.00%> (ø)
... and 2 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@hyj1991
Copy link
Member Author

hyj1991 commented Oct 30, 2022

另外,worker_threads 模式下 nyc 检测不到覆盖率导致的卡点问题也得想办法解决下

lib/app_worker.js Outdated Show resolved Hide resolved
@fengmk2 fengmk2 self-assigned this Oct 30, 2022
.eslintrc Show resolved Hide resolved
@hyj1991 hyj1991 marked this pull request as ready for review November 1, 2022 13:33
@hyj1991 hyj1991 requested a review from fengmk2 November 1, 2022 13:33
@fengmk2 fengmk2 merged commit 04a1e85 into eggjs:master Nov 2, 2022
@fengmk2
Copy link
Member

fengmk2 commented Nov 2, 2022

我发个 major 版本。

@fengmk2
Copy link
Member

fengmk2 commented Nov 2, 2022

2.0.0

Copy link
Member

@fengmk2 fengmk2 left a comment

Choose a reason for hiding this comment

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

尴尬。。。test/.DS_Store 也上传了

@hyj1991
Copy link
Member Author

hyj1991 commented Nov 2, 2022

尴尬。。。test/.DS_Store 也上传了

我来处理下

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.

None yet

2 participants