Skip to content

Conversation

@MaxGekk
Copy link
Member

@MaxGekk MaxGekk commented Apr 14, 2020

What changes were proposed in this pull request?

Replacing a floating-point division and Math.floor of Double value by Math.floorDiv for Long in DateTimeUtils.millisToDays().

Why are the changes needed?

The changes improve performance, in particular, it speeds up the conversion from java.sql.Date as the ported DateTimeBenchmark in MaxGekk#27 and comparisons in #28205 show.

Does this PR introduce any user-facing change?

No

How was this patch tested?

By existing test suite DateTimeUtilsSuite, DateFunctionsSuite and DateExpressionsSuite.

@kiszk
Copy link
Member

kiszk commented Apr 14, 2020

According to https://spark.apache.org/versioning-policy.html, I think that maintenance release will have only patches for bug fix, not for performance.

MAINTENANCE: Maintenance releases will occur more frequently and depend on specific patches introduced (e.g. bug fixes) and their urgency. In general these releases are designed to patch bugs. However, higher level libraries may introduce small features, such as a new algorithm, provided they are entirely additive and isolated from existing code paths. Spark core may not introduce any features.

@gatorsmile
Copy link
Member

I would close this PR.

@dongjoon-hyun
Copy link
Member

+1 for @kiszk and @gatorsmile 's advice.

@MaxGekk
Copy link
Member Author

MaxGekk commented Apr 14, 2020

@kiszk @gatorsmile @dongjoon-hyun Thank you for your feedback. I am closing this.

@MaxGekk MaxGekk closed this Apr 14, 2020
@SparkQA
Copy link

SparkQA commented Apr 14, 2020

Test build #121285 has finished for PR 28216 at commit b453aa2.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@MaxGekk MaxGekk deleted the optimize-fromJavaDate-2.4 branch June 5, 2020 19:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants