use Instrumentation#retransformClasses to hotswap

[hengyunabc]

#564

[skybber]

I have very similar idea, btw, where is the classToByteCodeMap cleared?

[skybber]

The second problem I see is atomicity. Each redefinition batch should be isolated from the others. In your solution, two consecutive hotswap() calls, R1 and R2, could redefine the same class, R1.C and R2.C, and only the last change (R2.C) would be processed by the redefinition. This could lead to a linkage error in DCEVM.

[hengyunabc]

1. The classToByteCodeMap does not need to be cleared. It represents the final version of the updated bytecode.
2. There indeed might be atomicity issues. However, since reTransform is asynchronous, try to resolve it would require implementing some complex counting mechanisms. Personally, I believe this would introduce unnecessary complexity. Moreover, if a mismatch in counts occurs, should we stop everything or continue proceed?

[hengyunabc]

If it is truly necessary to address atomicity issues, the following solution can be considered:

• Add some data in the bytecode to record which batch it belongs to. For example, add a meaningless annotation to the class, such as @Hotswap(byteCodeVersion = 111). In the Transformer, collect the information from the annotation, and then it will be possible to know the final bytecode version of each class.

[skybber]

May be there could a que of byte code be for each class in the weak map and the retransform method would pop bytecode from the que.

[hengyunabc]

May be there could a que of byte code be for each class in the weak map and the retransform method would pop bytecode from the que.

However, in extreme cases, confusion may still occur. For example, if another Java agent modifies the bytecode of the same class, triggering a redefine or reTransform, it becomes impossible to distinguish where the Transformer's callback was triggered from.

[skybber]

May be there could a que of byte code be for each class in the weak map and the retransform method would pop bytecode from the que.

However, in extreme cases, confusion may still occur. For example, if another Java agent modifies the bytecode of the same class, triggering a redefine or reTransform, it becomes impossible to distinguish where the Transformer's callback was triggered from.

I think that using queue+pop is better than overriding and retaining unnecessary bytecode in the map. When autoHotswap=true is enabled, there could be numerous redefinitions. Minecraft developers have reported that they can fill 500MB to 1GB of code cache with redefined classes.

[hengyunabc]

I think that using queue+pop is better than overriding and retaining unnecessary bytecode in the map. When autoHotswap=true is enabled, there could be numerous redefinitions. Minecraft developers have reported that they can fill 500MB to 1GB of code cache with redefined classes.

Retaining the final byte[] is necessary. Otherwise, if other Java agents trigger a redefine/retransform, it will cause the hotswap to no longer be effective.

One possible approach is to implement a disk-based cache, writing the byte[] to disk.

[skybber]

Interesting. I'll take some time to look into this. In the meantime, could you share more about your use case? I'm really curious.

[hengyunabc]

Interesting. I'll take some time to look into this. In the meantime, could you share more about your use case? I'm really curious.

We are currently facing an issue when using the hotswap agent:

Users are unsure whether a modified class has been successfully hot-updated. Although there may be some log outputs, users still do not know if the changes have taken effect.

We are considering allowing users to retrieve the runtime bytecode to determine if it has taken effect, for example, using: https://arthas.aliyun.com/en/doc/jad.html.

Therefore, we strongly prefer to use retransformClasses to update bytecode rather than redefineClasses.

[skybber]

If logging of redefinition is only reason for it then I recommend to use -Xlog:redefine+class*=info. It turns on basic redefinition info. If you have another use case for it, then we could add optional retranformation to HA, may be it can be enabled by a new HA option (retransform=true, both in hotswap-agent.properties and HA options), then we can use the patch as you proposed (with the option).