Skip to content

Conversation

@MaxGekk
Copy link
Member

@MaxGekk MaxGekk commented Jun 23, 2020

What changes were proposed in this pull request?

Set column width w/ benchmark names to maximum of either

  1. 40 (before this PR) or
  2. The length of benchmark name or
  3. Maximum length of cases names

Why are the changes needed?

To improve readability of benchmark results. For example, MakeDateTimeBenchmark.

Before:

make_timestamp():                         Best Time(ms)   Avg Time(ms)   Stdev(ms)    Rate(M/s)   Per Row(ns)   Relative
------------------------------------------------------------------------------------------------------------------------
prepare make_timestamp()                           3636           3673          38          0.3        3635.7       1.0X
make_timestamp(2019, 1, 2, 3, 4, 50.123456)             94             99           4         10.7          93.8      38.8X
make_timestamp(2019, 1, 2, 3, 4, 60.000000)             68             80          13         14.6          68.3      53.2X
make_timestamp(2019, 12, 31, 23, 59, 60.00)             65             79          19         15.3          65.3      55.7X
make_timestamp(*, *, *, 3, 4, 50.123456)            271            280          14          3.7         270.7      13.4X

After:

make_timestamp():                            Best Time(ms)   Avg Time(ms)   Stdev(ms)    Rate(M/s)   Per Row(ns)   Relative
---------------------------------------------------------------------------------------------------------------------------
prepare make_timestamp()                              3694           3745          82          0.3        3694.0       1.0X
make_timestamp(2019, 1, 2, 3, 4, 50.123456)             82             90           9         12.2          82.3      44.9X
make_timestamp(2019, 1, 2, 3, 4, 60.000000)             72             77           5         13.9          71.9      51.4X
make_timestamp(2019, 12, 31, 23, 59, 60.00)             67             71           5         15.0          66.8      55.3X
make_timestamp(*, *, *, 3, 4, 50.123456)               273            289          14          3.7         273.2      13.5X

Does this PR introduce any user-facing change?

No

How was this patch tested?

By re-generating benchmark results for MakeDateTimeBenchmark:

$ SPARK_GENERATE_BENCHMARK_FILES=1 build/sbt "sql/test:runMain org.apache.spark.sql.execution.benchmark.MakeDateTimeBenchmark"

in the environment:

Item Description
Region us-west-2 (Oregon)
Instance r3.xlarge
AMI ubuntu/images/hvm-ssd/ubuntu-bionic-18.04-amd64-server-20190722.1 (ami-06f2f779464715dc5)
Java OpenJDK 64-Bit Server VM 1.8.0_252 and OpenJDK 64-Bit Server VM 11.0.7+10

@MaxGekk MaxGekk changed the title [SPARK-32072][SQL][TESTS] Fix table formatting with benchmark results [SPARK-32072][CORE][TESTS] Fix table formatting with benchmark results Jun 23, 2020
@SparkQA
Copy link

SparkQA commented Jun 23, 2020

Test build #124415 has finished for PR 28906 at commit f0598b5.

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

@MaxGekk
Copy link
Member Author

MaxGekk commented Jun 23, 2020

@srowen @dongjoon-hyun Please, review this PR.

@SparkQA
Copy link

SparkQA commented Jun 23, 2020

Test build #124428 has finished for PR 28906 at commit e1a242e.

  • This patch fails build dependency tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@MaxGekk
Copy link
Member Author

MaxGekk commented Jun 23, 2020

jenkins, retest this, please

@SparkQA
Copy link

SparkQA commented Jun 23, 2020

Test build #124429 has finished for PR 28906 at commit e1a242e.

  • This patch fails build dependency tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@MaxGekk
Copy link
Member Author

MaxGekk commented Jun 23, 2020

jenkins, retest this, please

@SparkQA
Copy link

SparkQA commented Jun 23, 2020

Test build #124432 has finished for PR 28906 at commit e1a242e.

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

prepare make_date() 2951 3182 355 33.9 29.5 1.0X
make_date(2019, 9, 16) 2325 2415 101 43.0 23.2 1.3X
make_date(*, *, *) 4556 4573 17 21.9 45.6 0.6X
prepare make_date() 3309 3429 110 30.2 33.1 1.0X
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The formatting changes are positive of course. Most of the numbers didn't change much at all; these kinda of did, some much more than a few stdevs. Just checking, it was on the same hardware as in the heading?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think so, as the head is written by the benchmark framework automatically.

@cloud-fan
Copy link
Contributor

thanks, merging to master!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants