Skip to content

Conversation

@hassila
Copy link
Contributor

@hassila hassila commented Dec 11, 2023

Benchmark blog post draft as previously agreed, I put in December 21 as publication date as a placeholder, looking forward to your feedback.

@hassila hassila marked this pull request as ready for review December 12, 2023 13:34
Copy link
Contributor

@ktoso ktoso left a comment

Choose a reason for hiding this comment

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

LGTM, this is a great post (and project)!

Thank you for the post, I think it’s great — just a few minor “yay!” and rephrasing ideas inline.

Copy link
Contributor

@MahdiBM MahdiBM left a comment

Choose a reason for hiding this comment

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

Nice one :)

Just pointing out that the png should probably have a white/transparent background, but maintainers can let you know better if the black background is actually an issue or not 🙂

@finagolfin
Copy link
Member

Good post, but it would benefit from some motivation. Obviously this filled a need in your own work, maybe you could tell the story of how this package got started or some important incident where it caught some performance regression?

Such stories help motivate a post with real-world examples, would be great if you can add one or two.

@hassila
Copy link
Contributor Author

hassila commented Jan 8, 2024

Thanks @finagolfin - addressed the straightforward fixes in bd96a6e - will think if there's any good additional motivation/background we can add which doesn't just focus on our niche (tried to motivate it with the analogy to unit testing which seems fairly straightforward as an argument for the need for non-functional requirement validation).

@finagolfin
Copy link
Member

Your niche is fine too, as that is a big market.

@hassila
Copy link
Contributor Author

hassila commented Jan 9, 2024

Your niche is fine too, as that is a big market.

Ok, did a stab at 43290b5 with some background.

@hassila
Copy link
Contributor Author

hassila commented Feb 2, 2024

So I guess this is stuck at review with the blog post owners, or is there anything else to do?

@alexandersandberg
Copy link
Member

cc: @cthielen

@ktoso ktoso self-requested a review February 11, 2024 07:14
author: [hassila]
---

Have you ever encountered a _performance problem_ that slipped through to end users which resulted in a bug report? Do you systematically measure and validate performance metrics when making changes to your Swift package?
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 you could make the intro stronger by bringing up your point about the importance of unit and integration testing. The old adage "make it work, make it right, make it fast" is essentially what you're saying here. Putting that framing closer to the top will provide a better hook to encourage developers to read the rest of the post.

layout: post
published: true
date: 2023-12-21 10:00:00
title: Benchmarking in Swift
Copy link
Contributor

Choose a reason for hiding this comment

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

Any title alternatives that expand a bit more? e.g. "Introducing X …" or "A Quick Look …" etc.


Have you ever encountered a _performance problem_ that slipped through to end users which resulted in a bug report? Do you systematically measure and validate performance metrics when making changes to your Swift package?

In the realm of professional trading software, the role of a comprehensive benchmarking framework integrated with Continuous Integration (CI) parallels the importance of unit and integration testing. Just as unit and integration tests are essential for ensuring the functional correctness of software, benchmarking within a CI pipeline is crucial for continuously validating the non-functional aspects, such as high throughput, low latency, predictable performance and consistent resource usage. This is vital for maintaining the competitive edge in a fast-paced financial environment where the extreme market data rates and performance requirements means that even small variations in response time - on the scale of microseconds - can significantly impact trade outcomes.
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a good spot to take your "Have you ever encountered a performance problem" point, i.e. it's apparent in trading systems why performance is important, but it's important to the average developer too, who doesn't want poor or regressed performance.


In the realm of professional trading software, the role of a comprehensive benchmarking framework integrated with Continuous Integration (CI) parallels the importance of unit and integration testing. Just as unit and integration tests are essential for ensuring the functional correctness of software, benchmarking within a CI pipeline is crucial for continuously validating the non-functional aspects, such as high throughput, low latency, predictable performance and consistent resource usage. This is vital for maintaining the competitive edge in a fast-paced financial environment where the extreme market data rates and performance requirements means that even small variations in response time - on the scale of microseconds - can significantly impact trade outcomes.

After examining the existing infrastructure within the Swift ecosystem, we concluded that there were no existing solutions meeting our needs for multi-platform and rich metrics support, CI integration, and developer-friendliness. Therefore, we decided to develop a [Benchmark package](https://github.com/ordo-one/package-benchmark) and open-source it, believing it could help advance performance for the Swift community and benefit all of us.
Copy link
Contributor

Choose a reason for hiding this comment

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

Minor nit but "open source", not "open-source"


### The role of benchmarks

Constructing a set of benchmarks and consistently running them provides an indication when something is not performing as expected. Typically, other more specialized tools are employed for root-cause analysis to analyze and fix the problem (e.g., Instruments, DTrace, Heaptrack, Leaks, Sample, …).
Copy link
Contributor

Choose a reason for hiding this comment

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

It's not clear what "other more specialized tools" is talking about. I believe the point is that software analysis requires specialized tools, e.g. memory leak detectors for memory leaks, etc., and benchmarking fits in here for performance regressions - it's another specialized tool. Better to state that plainly.

}
```

### Performance metrics
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a good section but it feels like a condensed version of it belongs in your introduction, not after you start diving into the package.

alt="Sample text output for benchmarks"
src="/assets/images/benchmark-blog/Benchmark.png" />

### Key Benchmark workflows are supported
Copy link
Contributor

Choose a reason for hiding this comment

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

For better or worse, Swift.org is All Caps All The Time, e.g. "Key Benchmark Workflows Are Supported"


### Key Benchmark workflows are supported

* **[Automated Pull Request performance regression checks](https://swiftpackageindex.com/ordo-one/package-benchmark/documentation/benchmark/comparingbenchmarksci)** by comparing the performance metrics of a pull request with the main branch and having the PR workflow check fail if there is a regression according to absolute or relative thresholds specified per benchmark
Copy link
Contributor

Choose a reason for hiding this comment

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

Why are these bolded?

* **[Export of benchmark results in several formats](https://swiftpackageindex.com/ordo-one/package-benchmark/documentation/benchmark/exportingbenchmarks)** for analysis or visualization
* Running the Instruments profiler [on the benchmark suite executable directly from Xcode](https://github.com/ordo-one/package-benchmark/releases/tag/1.11.0)

### Community Adoption
Copy link
Contributor

Choose a reason for hiding this comment

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

Good info, but this doesn't need its own section. Recommend folding it into your conclusion.


### Summary

Performance is a key non-functional requirement for many software packages. Early and continuous benchmarking will help you to ship software with consistent performance and controlled resource usage that delights your users.
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd recommend a shorter wrap-up here with a call-to-action, e.g. "Give it a try". You don't want to be listing virtues here.

@hassila
Copy link
Contributor Author

hassila commented Mar 13, 2024

Thanks for the extensive feedback @cthielen, I have tried to address it in 4744b04

@cthielen
Copy link
Contributor

cthielen commented Mar 19, 2024

@hassila Thank you! Are you okay if I publish this (i.e. I'll adjust the date and merge) on Wednesday, 3/20 around 10am Pacific?

@hassila
Copy link
Contributor Author

hassila commented Mar 19, 2024

Thanks @cthielen that'd be perfectly fine, thanks.

@hassila hassila requested a review from federicobucchi as a code owner March 19, 2024 05:53
@cthielen
Copy link
Contributor

@swift-ci please test

@cthielen
Copy link
Contributor

@swift-ci please test

1 similar comment
@shahmishal
Copy link
Member

@swift-ci please test

@cthielen cthielen merged commit fcdf332 into swiftlang:main Mar 20, 2024
cthielen added a commit to Joannis/swift-org-website that referenced this pull request Mar 26, 2024
* Adding placeholder for blog post + updating author file

* First iteration

* Link to latest documentation

* Add link to Swift Certificates benchmark workflow

* Add link to Instruments integration

* Add link to community section at Swift forums

* Clarify language

* Fix SPI links to be version-independent

* Addrees PR feedback

* Update _posts/2023-12-15-benchmarks.md

Co-authored-by: Mahdi Bahrami <github@mahdibm.com>

* Address PR feedback, fix image background

* Fix wording

* Update _posts/2023-12-15-benchmarks.md

Co-authored-by: Alexander Sandberg <hi@alexandersandberg.com>

* Apply suggestions from code review

Co-authored-by: Alexander Sandberg <hi@alexandersandberg.com>

* Use comma consistently after e.g.

* Compress png image

* Feature -> Requirement

* Put placeholder date into the future again

* Add newline in the intro for nicer display

* Put placeholder date into the future again

* Address PR feedback

* Address PR feedback

* Add some background information on package origin as request

* Add Swift Package Manager as another package that have adopted Benchmark

* Fix authors for rebase

* Try to address PR feedback from cthielen

* Update publish date/time

---------

Co-authored-by: Mahdi Bahrami <github@mahdibm.com>
Co-authored-by: Alexander Sandberg <hi@alexandersandberg.com>
Co-authored-by: Christopher Thielen <77445+cthielen@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants