Skip to content

chore: Improve support - benchmarking suite#3014

Closed
coderfender wants to merge 7 commits into
apache:mainfrom
coderfender:improve_benchmarking_suite
Closed

chore: Improve support - benchmarking suite#3014
coderfender wants to merge 7 commits into
apache:mainfrom
coderfender:improve_benchmarking_suite

Conversation

@coderfender
Copy link
Copy Markdown
Contributor

@coderfender coderfender commented Dec 29, 2025

Which issue does this PR close?

Closes #3013

Rationale for this change

What changes are included in this PR?

How are these changes tested?

@coderfender coderfender changed the title ansi_suport_benchmarking chore: ANSI support benchmarking suite Dec 29, 2025
@coderfender
Copy link
Copy Markdown
Contributor Author

coderfender commented Dec 29, 2025

@andygrove , benchmarks supporting ANSI mode had no Try check and were failing when I run them on my local. This PR is to essentially add support for ANSI mode so that the flow doesn't error out. I also wanted to see advice and see if I could add changes to add more variety to the benchmarking data in this PR or should we perhaps create a new one ?

@coderfender coderfender marked this pull request as draft December 29, 2025 18:58
@coderfender
Copy link
Copy Markdown
Contributor Author

coderfender commented Dec 29, 2025

Refactoring to not pass along both SQLConf and isANSIEnabled

coderfender
coderfender commented Dec 29, 2025
View reviewed changes
Comment thread spark/src/test/scala/org/apache/spark/sql/benchmark/CometBenchmarkBase.scala
/** Runs function `f` with Comet on and off. */
final def runWithComet(name: String, cardinality: Long)(f: => Unit): Unit = {
val benchmark = new Benchmark(name, cardinality, output = output)

Copy link
Copy Markdown
Contributor Author

@coderfender coderfender Dec 29, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

this is no longer being used and needed as well IMO

@coderfender coderfender marked this pull request as ready for review December 29, 2025 19:31
@coderfender coderfender changed the title chore: ANSI support benchmarking suite chore: Improve support - benchmarking suite Dec 29, 2025
@coderfender coderfender mentioned this pull request Dec 29, 2025
Open
@andygrove
Copy link
Copy Markdown
Member

andygrove commented Jan 2, 2026

@andygrove , benchmarks supporting ANSI mode had no Try check and were failing when I run them on my local. This PR is to essentially add support for ANSI mode so that the flow doesn't error out. I also wanted to see advice and see if I could add changes to add more variety to the benchmarking data in this PR or should we perhaps create a new one ?

Disabling ANSI support makes sense for now. We can test the try_* methods to test the performance of the validation that happens in ANSI mode. If we want specific benchmarks for ANSI mode then we would need to ensure that we generate input data that will not result in errors. I don't see this is a priority though.

coderfender
coderfender commented Jan 3, 2026
View reviewed changes
Comment thread Makefile
sort)

# Run all discovered benchmarks
benchmark-all:
Copy link
Copy Markdown
Contributor Author

@coderfender coderfender Jan 3, 2026

Choose a reason for hiding this comment

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

Added convenient way to run all benchmark through make file

@coderfender
Copy link
Copy Markdown
Contributor Author

coderfender commented Jan 3, 2026

Thank you for the feedback @andygrove . I missed out in catching up with this thread before I went ahead and added ANSI safe data generation methods though . Also I updated make file to run all the benchmarks at once

@coderfender
Copy link
Copy Markdown
Contributor Author

coderfender commented Jan 8, 2026

@andygrove should we probably close this and come back to it later or rather fix above merge conflicts and move ahead ?

@coderfender coderfender closed this Feb 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Bug : Error running benchmarks with ANSI enabled

2 participants

@coderfender @andygrove