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

[Question] 以 Agent 模式启动时每次添加 ClassFileTransformer 都会尝试对所有已加载类进行 transform 的必要性 #439

Open
lemon0029 opened this issue Sep 10, 2023 · 4 comments
Assignees

Comments

@lemon0029
Copy link

我们以 Agent 的模式启动,premain 是在应用服务的 main 启动之前执行的,而 sandbox 的模块会在 premain 时就注册好 ClassFileTransformer, 是否有必要在这个时机尝试对所有已加载类进行 transform 呢?

大致观察了一下 sandbox 本身会加载大约 2000 个类,然后我们自己模块会加载 3000 个类,注册了大概 20 个 ClassFileTransformer, 实际上而言我们只希望对应用服务的某些类做增强,这时候目标类是还没有加载的。每次添加 Transformer 都需要对 5000 个类尝试做 transform, 其中类匹配因为封装了各种 ClassStructure 的缘故是比较慢的(我知道在 1.4.0 中已经做了相关优化,但是感知性能提升不大)。

核心问题还是 Agent 模式启动,注册每一个 ClassFileTransformer 之后到底有没有必要对所有已加载类尝试 transform, 我们使用时这里大部分情况都是不会增强到任何类的,所以是否可以考虑这个时机不要尝试增强(匹配)类来提升 premain 的性能,我们现在有多个模块启用,后面加载的模块永远要尝试匹配更多的类,而往往这时候又不会增强到任何类…

@Aresxue
Copy link

Aresxue commented Oct 17, 2023

类加载顺序不确定吧,可能ClassFileTransformer会晚于业务类的加载也可能会更早,如果较晚的不扫描所有已加载类那么字节码改写就永远无法生效了。而且一般ClassFileTransformer的注册为了提升启动速度经常是异步去做的比如jvm-sandbox-repeater

@lemon0029
Copy link
Author

类加载顺序不确定吧,可能ClassFileTransformer会晚于业务类的加载也可能会更早,如果较晚的不扫描所有已加载类那么字节码改写就永远无法生效了。而且一般ClassFileTransformer的注册为了提升启动速度经常是异步去做的比如jvm-sandbox-repeater

当然,类加载顺序是不确定的,但是如果是以 Agent 的模式启动的,此时在 premain 方法中应该是没有 BusinessClassLoader 的,即便在多个模块一起启动时加载了业务类,那也是通过 ModuleJarClassLoader 完成的,这并不会影响到实际应用程序,也就是说这时候应该有一个选项避免此时尝试去匹配类完成 loaded class transform. 另外你说的异步去做 EventWatcher 的注册,这个我们也是这么做的,但是容器环境如果是 1C / 2C 的情况,再加上 K8s 资源调度出现一些问题,这个时候异步的反倒没有同步的快...线下环境又不可能给每个容器分配 4C / 8C 的配置,所以我觉得核心还是要给 Agent 模式启动的模块提供只注册 EventWatcher (即 add ClassFileTransformer)但不尝试匹配 loaded class 来做 transform 的选项。

不知道 @oldmanpushcart 怎么看待这一点。另外关于 #444 提到的 BusinessClassLoader 移除的情况,此时用 AdviceListener API 时用到业务类的场景全部失效了,我们也只能通过自定义模块 Plugin & EventListener 来实现原有场景了,这让我们升级 v1.4.0 出现比较大的问题,然而这个在 release note 里面一点都没有提到...从设计上面来说,Event ClassLoader 在 Event Listener 里面用到不是很正常吗?

@Aresxue
Copy link

Aresxue commented Oct 23, 2023

类加载顺序不确定吧,可能ClassFileTransformer会晚于业务类的加载也可能会更早,如果较晚的不扫描所有已加载类那么字节码改写就永远无法生效了。而且一般ClassFileTransformer的注册为了提升启动速度经常是异步去做的比如jvm-sandbox-repeater

当然,类加载顺序是不确定的,但是如果是以 Agent 的模式启动的,此时在 premain 方法中应该是没有 BusinessClassLoader 的,即便在多个模块一起启动时加载了业务类,那也是通过 ModuleJarClassLoader 完成的,这并不会影响到实际应用程序,也就是说这时候应该有一个选项避免此时尝试去匹配类完成 loaded class transform. 另外你说的异步去做 EventWatcher 的注册,这个我们也是这么做的,但是容器环境如果是 1C / 2C 的情况,再加上 K8s 资源调度出现一些问题,这个时候异步的反倒没有同步的快...线下环境又不可能给每个容器分配 4C / 8C 的配置,所以我觉得核心还是要给 Agent 模式启动的模块提供只注册 EventWatcher (即 add ClassFileTransformer)但不尝试匹配 loaded class 来做 transform 的选项。

不知道 @oldmanpushcart 怎么看待这一点。另外关于 #444 提到的 BusinessClassLoader 移除的情况,此时用 AdviceListener API 时用到业务类的场景全部失效了,我们也只能通过自定义模块 Plugin & EventListener 来实现原有场景了,这让我们升级 v1.4.0 出现比较大的问题,然而这个在 release note 里面一点都没有提到...从设计上面来说,Event ClassLoader 在 Event Listener 里面用到不是很正常吗?

从使用者角度来说好像需要这么个选项,就是不知道框架愿不愿意做了,一旦开放这个选项就存在误用的可能性,到时候还是会到这边提issue。。。哈哈

@oldmanpushcart
Copy link
Collaborator

所以我觉得核心还是要给 Agent 模式启动的模块提供只注册 EventWatcher (即 add ClassFileTransformer)但不尝试匹配 loaded class 来做 transform 的选项。

我觉得你的建议非常合理,容我考虑和设计一下。

另外关于 #444 提到的 BusinessClassLoader 移除的情况,此时用 AdviceListener API 时用到业务类的场景全部失效了,我们也只能通过自定义模块 Plugin & EventListener 来实现原有场景了,这让我们升级 v1.4.0 出现比较大的问题,然而这个在 release note 里面一点都没有提到...

非常抱歉,没有在release note提起的确是我的工作不到位。再次表示道歉。而且这个移除是破坏了向下兼容性承诺的。但这特性确实没有准备好,还有一些我们自己内部都不能容忍的问题(比如一个类出现在不同的ClassLoader中时,BusinessClassLoader会选择第一个,而第一个未必是对的)。

我也知道这个特性对开发者非常友好,毕竟不用恶心的反射了。容我好好重新想想这个api应该如何设计。

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants