optimize: take action #198
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov Report
@@ Coverage Diff @@
## master #198 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 9 9
Lines 296 296
=========================================
Hits 296 296 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
legendecas
approved these changes
Oct 8, 2022
| #define CLEAR_DATA \ | ||
| DebugT(module_type, env_data->thread_id(), "<%s> %s dump_data cleared.", \ | ||
| notify_type.c_str(), unique_key.c_str()); \ | ||
| delete dump_data; |
There was a problem hiding this comment.
我也想过这里用智能指针，不过我不太熟悉怕反而写出来 bug，要不合并后你来看看是否可以重构成智能指针？
Merged
hyj1991
added a commit
that referenced
this pull request
Oct 22, 2022
Commits: - [2f700fd] docs: update readme (#207) - [f4eb960] feat: support node-v19.x (#206) - [333593e] chore: make test more reliable (#205) - [c574ca2] feat: save Elf BuildId in corefile. (#204) - [b779aa2] feat: support finish profiling before process exit (#203) - [6a1701f] refactor: replace dump actions static storages with EnvironmentData (#202) - [7608a86] fix: ignore clean error (#200) - [2a6eaba] chore: static dispatch on dumpaction (#199) - [7d3fbb2] optimize: take action (#198) PR-URL: #208
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
背景
早期 1.x 版本在尚未实现
worker_threads监控时，开发者的采样操作会通过requestInterrupt&uv_async_send同时通知js 主线程 —— 这意味着HandleAction在早期实现中会执行两次。因此旧版实现中在
HandleAction执行前校验了TransactionDone这个函数，通过${uuid}::${action}的唯一 key 设置状态判断当前的 action 是否已经由requestInterruptoruv_async_send执行过了，如果已经执行过，则清理 data 然后返回。而在最新的实现中，将
HandleAction放置到一个 lambda 回调中：并且这个回调函数会放入每一个 EnvData 对应的
std:list中，这样requestInterrupt&uv_async_send先触发的那一个会通过std::list::swap执行回调，后触发拿到的std::list为空不会再触发。这样
HandleAction只会执行一次，旧版本中设计的TransactionDone逻辑不再需要。优化
移除当前实现中为
HandleAction执行两次所做的冗余设计逻辑