HolyLow commented on PR #3177: URL: https://github.com/apache/celeborn/pull/3177#issuecomment-2771457621
> So let's use a different name for the first byte, in case someone might blindly add a new serialization layout when they want to add a new language client in the future. Maybe just V1, V2, or something that won't make the user think it's exclusive to some languages. @pan3793 Refactored the code as you suggested. The class name is SerdeVersion now, and the enums are V1 V2. > Just out of curiosity, why 0xFF is chosen for the new layout? No special reason, just pick a byte to represent it LOL as long as it is not 0xAC. You could pick one if you like, and I could refactor it. > I might miss some context, is there a doc that describes the layout? The "new serialization" is simply TransportMessage.toByteBuffer(), and there is no doc describes the toByteBuffer method. I think It is simple enough, and the doc lacking problem is still a java implementation problem and out of CIP-14's scope... > I think we should migrate all implementations to the new serialization layout to simplify the code eventually. In my experience in implementing the new CppClient, the existing java implementation's ser/de stack is really messy, and changing only one part would no affect the overall complexity. Actually, changing one part of ser/de and keeping rest the same only increases complexity. And the performance improvement is also under doubt without thorough testing and benchmarking. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
