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: fix nested gc callback #188

Merged
merged 5 commits into from
Aug 12, 2022
Merged

Conversation

theanarkh
Copy link
Contributor

fix nested gc callback.
Refs: nodejs/node#44058

src/commands/gcprofiler/gc_profiler.h Outdated Show resolved Hide resolved
src/commands/gcprofiler/gc_profiler.h Outdated Show resolved Hide resolved
@codecov
Copy link

codecov bot commented Aug 11, 2022

Codecov Report

Merging #188 (9348013) into master (86fc798) will not change coverage.
The diff coverage is n/a.

❗ Current head 9348013 differs from pull request most recent head baadd2c. Consider uploading reports for the commit baadd2c to get more accurate results

@@            Coverage Diff            @@
##            master      #188   +/-   ##
=========================================
  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

hyj1991 and others added 2 commits August 11, 2022 21:32
Co-authored-by: Chengzhong Wu <legendecas@gmail.com>
Co-authored-by: Chengzhong Wu <legendecas@gmail.com>
@hyj1991
Copy link
Member

hyj1991 commented Aug 11, 2022

@theanarkh 这样改后对于 nested 的 gc profiling 会丢失吧,不过我比较好奇什么情况下会出现 gc 嵌套的场景

@theanarkh
Copy link
Contributor Author

theanarkh commented Aug 11, 2022

Weak phantom callbacks run during the prologue or epilogue of major GCs

社区的同学这样说的。我测试的时候,kGCTypeProcessWeakCallbacks 总是嵌套到 kGCTypeMarkSweepCompact 里,所以我理解是时间会算到 kGCTypeMarkSweepCompact 里了。我没有找到复现的代码,是在公司的项目里发现的,目前这样改动是最小的,否则遇到嵌套 GC 的话,目前的实现应该是会出现文件数据错乱?我在公司项目里解决方法是在 GC start 和 end 的时候都写一个完整的 json 对象到文件里,然后解析的时候把文件里的对象数组当作一个栈,匹配同类型的 start 和 end 计算出耗时,这样看起来是能区分出 kGCTypeProcessWeakCallbacks 类型的 GC 耗时。

@theanarkh
Copy link
Contributor Author

image

@legendecas
Copy link
Member

kGCTypeProcessWeakCallbacks 是通过 SecondPassCallback 触发的,这个目前有两种方式触发:1. 通过 platform task 延迟触发,这种不会被 nest 到其他类型的 GC 里,2. 强制或者是内存急迫时的 GC,这种时候 SecondPassCallback 是同步调用的会会嵌套在其他类型的 GC 里。目前除了 node-api 里的 finalizer 是 SecondPassCallback 中调用的,其他的应该比较少见。

这个改动应该没什么问题。

@legendecas legendecas merged commit 92f651c into X-Profiler:master Aug 12, 2022
@legendecas
Copy link
Member

谢谢提交修复!

@hyj1991 hyj1991 mentioned this pull request Oct 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants