Bill commented on a change in pull request #5344:
URL: https://github.com/apache/geode/pull/5344#discussion_r450504984
##########
File path:
geode-serialization/src/main/java/org/apache/geode/internal/serialization/Versioning.java
##########
@@ -26,11 +26,40 @@
public class Versioning {
private Versioning() {}
+ /**
+ * Make a VersionOrdinal for the short ordinal value.
+ *
+ * If the short ordinal represents a known version (Version) then return
+ * that instead of constructing a new VersionOrdinal.
+ *
+ * @return a known version (Version) if possible, otherwise a VersionOrdinal.
+ */
public static VersionOrdinal getVersionOrdinal(final short ordinal) {
- try {
- return Version.fromOrdinal(ordinal);
- } catch (final UnsupportedSerializationVersionException e) {
- return new VersionOrdinalImpl(ordinal);
+ final Version knownVersion = Version.getKnownVersion(ordinal, null);
+ if (knownVersion == null) {
+ return new UnknownVersion(ordinal);
+ } else {
+ return knownVersion;
+ }
+ }
+
+ /**
+ * Return the known version (Version) for the VersionOrdinal, if possible.
+ * Otherwise return the returnWhenUnknown Version. This method essentially
+ * downcasts a {@link VersionOrdinal} to a known version {@link Version}
+ *
+ * @param anyVersion came from a call to {@link #getVersionOrdinal(short)}
or this
+ * method
+ * @param returnWhenUnknown will be returned if anyVersion does not represent
+ * a known version
+ * @return a known version
+ */
+ public static Version getKnownVersion(final VersionOrdinal anyVersion,
+ Version returnWhenUnknown) {
+ if (anyVersion instanceof Version) {
Review comment:
It might be more efficient. Or it might not be. What is certain, is that
it would add a method to the `VersionOrdinal` interface.
My thinking is that if we write our deserialization code correctly then we
need to do this version determination at most once when establishing a network
connection (to detect the serialization version of the counterparty) or once
when reading from a serialized file on disk (to detect the serialization
version of the content in the file). Establishing a network connection or
opening a file for reading take on the order of a millisecond whereas
`instanceof` will take on the order of nanoseconds.
If you find my reasoning sound, I'd like to leave the `instanceof` call here
instead of adding a predicate to `VersionOrdinal`.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
