Closed abissell closed 1 year ago
Once I dug into this a bit, I think I can understand better why LocalDateTime
was chosen and why it makes sense to stick with it.
Under the hood, the various java.time
classes are represented as follows:
Instant
96 bits: long
seconds (since start of Unix epoch), int
nanos
LocalTime
56 bits: byte
hour, byte
minute, byte
second, int
nanos
LocalDate
64 bits: int
year, short
month, short
day
LocalDateTime
120 bits: LocalDate
and LocalTime
When these values are (de)-serialized to a FIX message, the library is able to read/write each element it needs directly, rather than extracting a year, month, and day from a Unix epoch timestamp. Additionally, while the Javadocs officially represent that a LocalDateTime
cannot represent a point on the UTC timeline, in practice clients should always be using the UTC offset anyway, and it should not introduce any genuine issues. Once these are converted to value objects by Project Valhalla, there should also be no performance difference between a 96 bit and 120 bit representation, since they'll likely both be padded to 128 bits anyway.
Withdrawing this issue.
Discussed in https://github.com/quickfix-j/quickfixj/discussions/629
The
java.time.Instant
data type would be better suited to represent FIX UTC timestamp fields than the currently usedLocalDateTime
. Some extensive discussion of the differences can be found here: https://stackoverflow.com/questions/32437550/whats-the-difference-between-instant-and-localdatetimeI imagine this would be a breaking change so probably would need to wait on a major version bump.