You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@fury.apache.org by "LiangliangSui (via GitHub)" <gi...@apache.org> on 2024/04/08 16:10:06 UTC
[I] [Java] Optimize ClassResolver#register bytecode size [incubator-fury]
LiangliangSui opened a new issue, #1482:
URL: https://github.com/apache/incubator-fury/issues/1482
## Is your feature request related to a problem? Please describe.
`ClassResolver#register` bytecode size is 341 bytes，hot method too big
## Describe the solution you'd like
Reduce `ClassResolver#register` bytecode size so that `ClassResolver#register` hot method can still be inlined.
## Additional context
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@fury.apache.org.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@fury.apache.org
For additional commands, e-mail: commits-help@fury.apache.org
Re: [I] [Java] Optimize ClassResolver#register bytecode size [incubator-fury]
Posted by "chaokunyang (via GitHub)" <gi...@apache.org>.
chaokunyang commented on issue #1482:
URL: https://github.com/apache/incubator-fury/issues/1482#issuecomment-2053672243
Is this the most timeconsuming part of Fury creation?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@fury.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@fury.apache.org
For additional commands, e-mail: commits-help@fury.apache.org
Re: [I] [Java] Optimize ClassResolver#register bytecode size [incubator-fury]
Posted by "LiangliangSui (via GitHub)" <gi...@apache.org>.
LiangliangSui commented on issue #1482:
URL: https://github.com/apache/incubator-fury/issues/1482#issuecomment-2054033017
> The thing is that it should not be a hot method. If it does, it means users created lots of Fury, which is not allowed.
Got it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@fury.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@fury.apache.org
For additional commands, e-mail: commits-help@fury.apache.org
Re: [I] [Java] Optimize ClassResolver#register bytecode size [incubator-fury]
Posted by "chaokunyang (via GitHub)" <gi...@apache.org>.
chaokunyang commented on issue #1482:
URL: https://github.com/apache/incubator-fury/issues/1482#issuecomment-2043150748
The registration is only executed for Fury init. It's not on the critical path. Do we need to optimize this?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@fury.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@fury.apache.org
For additional commands, e-mail: commits-help@fury.apache.org
Re: [I] [Java] Optimize ClassResolver#register bytecode size [incubator-fury]
Posted by "LiangliangSui (via GitHub)" <gi...@apache.org>.
LiangliangSui commented on issue #1482:
URL: https://github.com/apache/incubator-fury/issues/1482#issuecomment-2054033534
> @LiangliangSui I will start the work of meta string in https://fury.apache.org/docs/specification/fury_xlang_serialization_spec/#meta-string to reduce classname serialization space cost before we make release for #1389 . If you'd like to optimize FUry creation, I think maybe we can go that way to optimize the most time-consuming part first
That's really cool, I'd be very interested in doing it and learning from you at the same time.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@fury.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@fury.apache.org
For additional commands, e-mail: commits-help@fury.apache.org
Re: [I] [Java] Optimize ClassResolver#register bytecode size [incubator-fury]
Posted by "chaokunyang (via GitHub)" <gi...@apache.org>.
chaokunyang commented on issue #1482:
URL: https://github.com/apache/incubator-fury/issues/1482#issuecomment-2054030986
After I optimized register in https://github.com/apache/incubator-fury/issues/1510 , I found Map.put is hte bottleneck now Maybe we should optimize that first.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@fury.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@fury.apache.org
For additional commands, e-mail: commits-help@fury.apache.org
Re: [I] [Java] Optimize ClassResolver#register bytecode size [incubator-fury]
Posted by "LiangliangSui (via GitHub)" <gi...@apache.org>.
LiangliangSui commented on issue #1482:
URL: https://github.com/apache/incubator-fury/issues/1482#issuecomment-2043177401
Although it is only executed during init, it is also identified as hot code. When I benchmark, I think we can try to optimize it.
I want to try to extract the following code in ClassResolver#register into a function (because it is not often called) and see how much the bytecode size can be reduced.
```java
if (registeredId2ClassInfo.length <= id) {
// extract a method below.
ClassInfo[] tmp = new ClassInfo[(id + 1) * 2];
System.arraycopy(registeredId2ClassInfo, 0, tmp, 0, registeredId2ClassInfo.length);
registeredId2ClassInfo = tmp;
}
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@fury.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@fury.apache.org
For additional commands, e-mail: commits-help@fury.apache.org
Re: [I] [Java] Optimize ClassResolver#register bytecode size [incubator-fury]
Posted by "LiangliangSui (via GitHub)" <gi...@apache.org>.
LiangliangSui commented on issue #1482:
URL: https://github.com/apache/incubator-fury/issues/1482#issuecomment-2053657741
I extracted a piece of logic that is not executed frequently into a function to reduce the bytecode size of the register method. The extracted function is as follows:
```java
public void register(Class<?> cls, int classId) {
......
short id = (short) classId;
if (id < registeredId2ClassInfo.length && registeredId2ClassInfo[id] != null) {
throw new IllegalArgumentException(
String.format(
"Class %s with id %s has been registered, registering class %s with same id are not allowed.",
registeredId2ClassInfo[id].getCls(), id, cls.getName()));
}
extRegistry.registeredClassIdMap.put(cls, id);
if (registeredId2ClassInfo.length <= id) {
ensure(id);
}
......
}
private void ensure(short id) {
ClassInfo[] tmp = new ClassInfo[(id + 1) * 2];
System.arraycopy(registeredId2ClassInfo, 0, tmp, 0, registeredId2ClassInfo.length);
registeredId2ClassInfo = tmp;
}
```
Then the `register` function bytecode is reduced from 341 to 314, which is lower than `-XX:+FreqInlineSize(325)`, and is successfully inlined.
I will upload a PR later to do this.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@fury.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@fury.apache.org
For additional commands, e-mail: commits-help@fury.apache.org
Re: [I] [Java] Optimize ClassResolver#register bytecode size [incubator-fury]
Posted by "LiangliangSui (via GitHub)" <gi...@apache.org>.
LiangliangSui commented on issue #1482:
URL: https://github.com/apache/incubator-fury/issues/1482#issuecomment-2054029527
We have optimized `register` in https://github.com/apache/incubator-fury/issues/1510, but I think by reducing the generated bytecode size the same can be done by inlining `register` when it becomes a hot method. Also useful for `register`.
@chaokunyang WDYT?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@fury.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@fury.apache.org
For additional commands, e-mail: commits-help@fury.apache.org
Re: [I] [Java] Optimize ClassResolver#register bytecode size [incubator-fury]
Posted by "LiangliangSui (via GitHub)" <gi...@apache.org>.
LiangliangSui closed issue #1482: [Java] Optimize ClassResolver#register bytecode size
URL: https://github.com/apache/incubator-fury/issues/1482
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@fury.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@fury.apache.org
For additional commands, e-mail: commits-help@fury.apache.org
Re: [I] [Java] Optimize ClassResolver#register bytecode size [incubator-fury]
Posted by "chaokunyang (via GitHub)" <gi...@apache.org>.
chaokunyang commented on issue #1482:
URL: https://github.com/apache/incubator-fury/issues/1482#issuecomment-2053522990
@LiangliangSui do we have any updates on this?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@fury.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@fury.apache.org
For additional commands, e-mail: commits-help@fury.apache.org
Re: [I] [Java] Optimize ClassResolver#register bytecode size [incubator-fury]
Posted by "chaokunyang (via GitHub)" <gi...@apache.org>.
chaokunyang commented on issue #1482:
URL: https://github.com/apache/incubator-fury/issues/1482#issuecomment-2054031927
@LiangliangSui I will start the work of meta string in https://fury.apache.org/docs/specification/fury_xlang_serialization_spec/#meta-string to reduce classname serialization space cost before we make release for #1389 . If you'd like to optimize FUry creation, I think maybe we can go that way to optimize the most time-consuming part first
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@fury.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@fury.apache.org
For additional commands, e-mail: commits-help@fury.apache.org
Re: [I] [Java] Optimize ClassResolver#register bytecode size [incubator-fury]
Posted by "chaokunyang (via GitHub)" <gi...@apache.org>.
chaokunyang commented on issue #1482:
URL: https://github.com/apache/incubator-fury/issues/1482#issuecomment-2053672133
The Fury creation is not frequent, the Jvm compiler is still in c1 stage. The inline threshold is 34 byte in such cases.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@fury.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@fury.apache.org
For additional commands, e-mail: commits-help@fury.apache.org
Re: [I] [Java] Optimize ClassResolver#register bytecode size [incubator-fury]
Posted by "chaokunyang (via GitHub)" <gi...@apache.org>.
chaokunyang commented on issue #1482:
URL: https://github.com/apache/incubator-fury/issues/1482#issuecomment-2044163300
@LiangliangSui You can also use async-profiler to cpature a flame graph to identify the most time-consuming part
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@fury.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@fury.apache.org
For additional commands, e-mail: commits-help@fury.apache.org
Re: [I] [Java] Optimize ClassResolver#register bytecode size [incubator-fury]
Posted by "LiangliangSui (via GitHub)" <gi...@apache.org>.
LiangliangSui commented on issue #1482:
URL: https://github.com/apache/incubator-fury/issues/1482#issuecomment-2053682243
> The Fury creation is not frequent, the Jvm compiler is still in c1 stage. The inline threshold is 34 byte in such cases.
What we can optimize is that when `register` becomes a hot method, it cannot be inlined because its bytecode is larger than `-XX:+FreqInlineSize(325)`.
`-XX:+MaxInlineSize(35)` should be the maximum limit that can be inlined before `register` becomes a hotspot method.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@fury.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@fury.apache.org
For additional commands, e-mail: commits-help@fury.apache.org
Re: [I] [Java] Optimize ClassResolver#register bytecode size [incubator-fury]
Posted by "LiangliangSui (via GitHub)" <gi...@apache.org>.
LiangliangSui commented on issue #1482:
URL: https://github.com/apache/incubator-fury/issues/1482#issuecomment-2053683564
> Is this the most timeconsuming part of Fury creation?
During the Fury creation process, the most time-consuming thing should be the `org.apache.fury.resolver.ClassResolver#addDefaultSerializers` function, not the `register` function.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@fury.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@fury.apache.org
For additional commands, e-mail: commits-help@fury.apache.org
Re: [I] [Java] Optimize ClassResolver#register bytecode size [incubator-fury]
Posted by "chaokunyang (via GitHub)" <gi...@apache.org>.
chaokunyang commented on issue #1482:
URL: https://github.com/apache/incubator-fury/issues/1482#issuecomment-2054030736
The thing is that it should not be a hot method. If it does, it means users created lots of Fury, which is not allowed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@fury.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@fury.apache.org
For additional commands, e-mail: commits-help@fury.apache.org