Refactoring of Timestamp #283

plokhotnyuk · 2022-06-27T14:21:25Z

Goals for refactoring of smithy4s.Timestamp:

Fix construction/parsing/serialization/formatting to be consistent between platforms and follow the protocol standards
Minimize the memory footprint for all platforms
More efficient parsing/formatting from/to string for all supported formats
More efficient parsing/serialization from/to JSON for all supported formats

The range of allowed values reduced to be from 0000-01-01T00:00:00Z to 9999-12-31T23:59:59Z.

Below are results of benchmarks for parsing and serialization of array of 128 java.time.Instant values using JSON codec for smithy4s.Timestamp on Intel® Core™ i9-11900H with JDK 17.

Before:

[info] Benchmark                                                             (size)   Mode  Cnt       Score       Error   Units
[info] ArrayOfInstantsReading.smithy4sJson                                      128  thrpt    5   11671.896 ±   147.913   ops/s
[info] ArrayOfInstantsReading.smithy4sJson:·gc.alloc.rate                       128  thrpt    5    2071.824 ±    26.405  MB/sec
[info] ArrayOfInstantsReading.smithy4sJson:·gc.alloc.rate.norm                  128  thrpt    5  279281.183 ±     0.073    B/op
[info] ArrayOfInstantsReading.smithy4sJson:·gc.churn.PS_Eden_Space              128  thrpt    5    2046.870 ±     0.376  MB/sec
[info] ArrayOfInstantsReading.smithy4sJson:·gc.churn.PS_Eden_Space.norm         128  thrpt    5  275919.803 ±  3493.766    B/op
[info] ArrayOfInstantsReading.smithy4sJson:·gc.churn.PS_Survivor_Space          128  thrpt    5       0.100 ±     0.274  MB/sec
[info] ArrayOfInstantsReading.smithy4sJson:·gc.churn.PS_Survivor_Space.norm     128  thrpt    5      13.484 ±    37.148    B/op
[info] ArrayOfInstantsReading.smithy4sJson:·gc.count                            128  thrpt    5      20.000              counts
[info] ArrayOfInstantsReading.smithy4sJson:·gc.time                             128  thrpt    5      10.000                  ms
[info] ArrayOfInstantsWriting.smithy4sJson                                      128  thrpt    5   37616.648 ±   939.227   ops/s
[info] ArrayOfInstantsWriting.smithy4sJson:·gc.alloc.rate                       128  thrpt    5    2311.128 ±    57.287  MB/sec
[info] ArrayOfInstantsWriting.smithy4sJson:·gc.alloc.rate.norm                  128  thrpt    5   96667.132 ±     1.520    B/op
[info] ArrayOfInstantsWriting.smithy4sJson:·gc.churn.PS_Eden_Space              128  thrpt    5    2251.551 ±  1079.548  MB/sec
[info] ArrayOfInstantsWriting.smithy4sJson:·gc.churn.PS_Eden_Space.norm         128  thrpt    5   94203.531 ± 46229.876    B/op
[info] ArrayOfInstantsWriting.smithy4sJson:·gc.churn.PS_Survivor_Space          128  thrpt    5       0.071 ±     0.166  MB/sec
[info] ArrayOfInstantsWriting.smithy4sJson:·gc.churn.PS_Survivor_Space.norm     128  thrpt    5       2.954 ±     6.901    B/op
[info] ArrayOfInstantsWriting.smithy4sJson:·gc.count                            128  thrpt    5      22.000              counts
[info] ArrayOfInstantsWriting.smithy4sJson:·gc.time                             128  thrpt    5      12.000                  ms

After:

[info] Benchmark                                                             (size)   Mode  Cnt       Score       Error   Units
[info] ArrayOfInstantsReading.smithy4sJson                                      128  thrpt    5  110357.823 ±  2238.914   ops/s
[info] ArrayOfInstantsReading.smithy4sJson:·gc.alloc.rate                       128  thrpt    5    1908.421 ±    38.816  MB/sec
[info] ArrayOfInstantsReading.smithy4sJson:·gc.alloc.rate.norm                  128  thrpt    5   27208.923 ±     0.419    B/op
[info] ArrayOfInstantsReading.smithy4sJson:·gc.churn.PS_Eden_Space              128  thrpt    5    1944.579 ±   881.102  MB/sec
[info] ArrayOfInstantsReading.smithy4sJson:·gc.churn.PS_Eden_Space.norm         128  thrpt    5   27726.767 ± 12634.748    B/op
[info] ArrayOfInstantsReading.smithy4sJson:·gc.churn.PS_Survivor_Space          128  thrpt    5       0.075 ±     0.193  MB/sec
[info] ArrayOfInstantsReading.smithy4sJson:·gc.churn.PS_Survivor_Space.norm     128  thrpt    5       1.068 ±     2.752    B/op
[info] ArrayOfInstantsReading.smithy4sJson:·gc.count                            128  thrpt    5      19.000              counts
[info] ArrayOfInstantsReading.smithy4sJson:·gc.time                             128  thrpt    5       8.000                  ms
[info] ArrayOfInstantsWriting.smithy4sJson                                      128  thrpt    5  145260.769 ±  5517.860   ops/s
[info] ArrayOfInstantsWriting.smithy4sJson:·gc.alloc.rate                       128  thrpt    5    2322.867 ±    88.449  MB/sec
[info] ArrayOfInstantsWriting.smithy4sJson:·gc.alloc.rate.norm                  128  thrpt    5   25160.847 ±     0.378    B/op
[info] ArrayOfInstantsWriting.smithy4sJson:·gc.churn.PS_Eden_Space              128  thrpt    5    2353.869 ±  1079.483  MB/sec
[info] ArrayOfInstantsWriting.smithy4sJson:·gc.churn.PS_Eden_Space.norm         128  thrpt    5   25490.408 ± 11403.692    B/op
[info] ArrayOfInstantsWriting.smithy4sJson:·gc.churn.PS_Survivor_Space          128  thrpt    5       0.121 ±     0.191  MB/sec
[info] ArrayOfInstantsWriting.smithy4sJson:·gc.churn.PS_Survivor_Space.norm     128  thrpt    5       1.307 ±     2.058    B/op
[info] ArrayOfInstantsWriting.smithy4sJson:·gc.count                            128  thrpt    5      23.000              counts
[info] ArrayOfInstantsWriting.smithy4sJson:·gc.time                             128  thrpt    5      12.000                  ms

Baccata · 2022-06-28T08:34:22Z

modules/core/src-jvm/smithy4s/Timestamp.scala

+
+  private[this] def appendNano(nano: Int, s: java.lang.StringBuilder): Unit =
+    if (nano != 0) {
+      val y1 = nano * 1441151881L // Based on James Anhalt's algorithm for 9 digits: https://jk-jeon.github.io/posts/2022/02/jeaiii-algorithm/


thanks for inlining a reference to the algorithm !

Baccata · 2022-06-28T09:12:31Z

modules/core/src-jvm/smithy4s/Timestamp.scala

+  }
+
+  /** Has platform specific implementation */
+  private[this] def formatHTTPDate: String = {


out of curiosity, what makes it platform specific ? From a quick look, it seems like it would cross-compile, so I presume it's a case that this implementation would not be optimal on JS.

If I want to cross-publish to Native, will I be able to share the sources between JVM and Native ?

Yes, the current draft is not optimized for any of platforms.

I'm going to have own Timestamp sources for each platform.

If JVM version should run on JDK 8 then it will be the same as for Scala Native.

If JVM version should run on JDK 8 then it will be the same as for Scala Native.

Coo. I don't want to assume you'll have time to dedicate to implementing both a JDK 11+ and a SN-compatible version, so let's do JDK 8 for now

Baccata · 2022-06-28T09:26:35Z

I'm happy with the direction this is taking 👍

…latforms accordingly

…or JVM platform

…m/to JSON for the EPOCH_SECONDS format

Baccata

Thank you so much for this 👍

kubukoz · 2022-07-12T17:26:15Z

The range of allowed values reduced to be from 0000-01-01T00:00:00Z to 9999-12-31T23:59:59Z.

@plokhotnyuk can you share more context on this? Looks like the default Abitrary[Instant] from scalacheck is now generating values that fall outside this range, so any user code relying on these arbitraries will break

plokhotnyuk · 2022-07-12T17:42:07Z

@plokhotnyuk can you share more context on this? Looks like the default Abitrary[Instant] from scalacheck is now generating values that fall outside this range, so any user code relying on these arbitraries will break

The reason of decreasing the range of allowed values for smithy4s.Timestamp is preventing of loosing information during serialization to standard formats.

Baccata reviewed Jun 28, 2022

View reviewed changes

plokhotnyuk force-pushed the refactoring-of-timestamp branch 6 times, most recently from 217bb65 to cead116 Compare June 29, 2022 08:53

Refactoring of Timestamp

23ee362

plokhotnyuk force-pushed the refactoring-of-timestamp branch from cead116 to 23ee362 Compare June 29, 2022 09:03

Fix Pizza client test failures

e3e768d

plokhotnyuk marked this pull request as ready for review June 29, 2022 09:43

plokhotnyuk added 5 commits June 29, 2022 12:16

Forbid negative years

63d2bcb

Don't skip validation of epochSecond values in the nowUTC call

db6dedb

Rollback Timestamp.fromEpochSecond

c203811

Add Timestamp.fromDate and Timestamp.fromInstant for JS and JVM p…

f41cc92

…latforms accordingly

Add Timestamp.toOffsetDateTime and Timestamp.fromOffsetDateTime f…

d117b4d

…or JVM platform

plokhotnyuk force-pushed the refactoring-of-timestamp branch 2 times, most recently from 2504300 to 65fc57f Compare June 29, 2022 12:34

Don't catch Throwable + add missing tests

ccc6f3e

plokhotnyuk force-pushed the refactoring-of-timestamp branch from 65fc57f to ccc6f3e Compare June 29, 2022 12:44

Add missing check for redundant chars

6d393bc

plokhotnyuk force-pushed the refactoring-of-timestamp branch 2 times, most recently from 47639b2 to 458f4f0 Compare June 30, 2022 07:05

More efficient parsing/serialization of Timestamp formats from/to JSON

76a53fd

plokhotnyuk force-pushed the refactoring-of-timestamp branch from 458f4f0 to 76a53fd Compare June 30, 2022 07:13

Fix loosing of timestamp nanoseconds during parsing/serialization fro…

57c26b2

…m/to JSON for the EPOCH_SECONDS format

Baccata approved these changes Jun 30, 2022

View reviewed changes

Baccata merged commit d9b4f77 into disneystreaming:main Jun 30, 2022

plokhotnyuk deleted the refactoring-of-timestamp branch June 30, 2022 10:10

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactoring of Timestamp #283

Refactoring of Timestamp #283

plokhotnyuk commented Jun 27, 2022 •

edited

Loading

Baccata Jun 28, 2022

Baccata Jun 28, 2022

plokhotnyuk Jun 28, 2022 •

edited

Loading

Baccata Jun 28, 2022

Baccata commented Jun 28, 2022

Baccata left a comment

kubukoz commented Jul 12, 2022

plokhotnyuk commented Jul 12, 2022

Refactoring of Timestamp #283

Refactoring of Timestamp #283

Conversation

plokhotnyuk commented Jun 27, 2022 • edited Loading

Baccata Jun 28, 2022

Choose a reason for hiding this comment

Baccata Jun 28, 2022

Choose a reason for hiding this comment

plokhotnyuk Jun 28, 2022 • edited Loading

Choose a reason for hiding this comment

Baccata Jun 28, 2022

Choose a reason for hiding this comment

Baccata commented Jun 28, 2022

Baccata left a comment

Choose a reason for hiding this comment

kubukoz commented Jul 12, 2022

plokhotnyuk commented Jul 12, 2022

plokhotnyuk commented Jun 27, 2022 •

edited

Loading

plokhotnyuk Jun 28, 2022 •

edited

Loading