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

Upgrade the async profiler library file version #2903

Merged
merged 3 commits into from
Sep 18, 2024

Conversation

ZhaoGuorui666
Copy link
Contributor

根据async profiler pr#733

  • 在该pr中async profiler构建一个了同时适用于glibc和musl的单一版本,所以从原来的五个库文件,到现在的三个库文件;
  • macos下的库文件变为.dylib格式;
  • ProfilerCommand中对不同环境的判断 进行相应更改;
  • workflows下的build-async-profiler.yml文件进行相应的更改;

@hengyunabc
Copy link
Collaborator

这个 pr 同时提交了二进制文件,这个应该由 build-async-profiler.yml 生成,它会自动提交到 master 分支上。这个可以保证安全性。

另外,glibc和musl的单一版本 这个有经过测试了不?

@hengyunabc
Copy link
Collaborator

另外, 之前因为 async-profiler 官方打包结果里没有支持 musl ,所以我们才自己打包了 musl 的结果: #2481

是不是全部直接引用官方最新的 https://github.com/async-profiler/async-profiler/releases/tag/v3.0 打包结果就可以了?

如果是的话,build-async-profiler.yml的流程可以先暂时保留,我们先直接用官方的构建结果。

@ZhaoGuorui666
Copy link
Contributor Author

测试结果:直接引用官方最新的打包结果可以正常运行;
glibc和musl的测试结果:
image
image
macos的测试结果:
image

if (OSUtils.isX86_64() && OSUtils.isMuslLibc()) {
profilerSoPath = "async-profiler/libasyncProfiler-linux-musl-x64.so";
} else if(OSUtils.isX86_64()){
if ((OSUtils.isX86_64() && OSUtils.isMuslLibc()) || OSUtils.isX86_64()) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

这个代码判断有多余的地方,实际上只判断 OSUtils.isX86_64() 就可以了。 下面 arm 平台的也是。

Copy link
Contributor Author

Choose a reason for hiding this comment

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

已修改

@hengyunabc hengyunabc added this to the 4.0.2 milestone Sep 18, 2024
@hengyunabc hengyunabc merged commit 2b6d331 into alibaba:master Sep 18, 2024
14 checks passed
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.

2 participants