chesnokoff commented on code in PR #12634:
URL: https://github.com/apache/ignite/pull/12634#discussion_r2730556935
##########
modules/core/src/main/java/org/apache/ignite/spi/discovery/tcp/TcpDiscoveryIoSession.java:
##########
@@ -163,63 +176,123 @@ void writeMessage(TcpDiscoveryAbstractMessage msg)
throws IgniteCheckedException
* @throws IgniteCheckedException If deserialization fails.
*/
<T> T readMessage() throws IgniteCheckedException, IOException {
- byte serMode = (byte)in.read();
+ return reader.readMessage();
+ }
- if (JAVA_SERIALIZATION == serMode)
- return U.unmarshal(spi.marshaller(), in, clsLdr);
+ /** Switches reader to fast mode after initial validated read. */
+ public void switchToFastReader() {
+ reader = fastReader;
+ }
- try {
- if (MESSAGE_SERIALIZATION != serMode) {
Review Comment:
Initially, `detectJavaObjectStreamHeader` was placed after `detectSslAlert`.
The suggestion was to avoid checking `serMode` after the connection is
established.
To achieve this, we need a way to notify session that these checks are no
longer required. The current approach is that higher-level code calls
`switchToFastReader` and it swaps the reader implementation. As a result, we
eliminate a single if check.
However, to me this looks like a very minor optimization, because I/O
dominates the cost compared to a simple conditional. Moreover, it seems to
break the DIP, since higher-level code now has to be aware of implementation
details of TcpDiscoveryIoSession
--
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]
