-
Notifications
You must be signed in to change notification settings - Fork 128
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Benchmarks with hyperfine #129
Comments
I ran a quick benchmark between tealdeer, the C client, and the npm client. I'll run a more exhaustive benchmark when (if ?) I get some time.
|
Hi, I came here after reading #145. I have written a TLDR client called Outfieldr that, according to my own hyperfine benchmarks, runs around 30 times faster than Tealdeer. Here is the hyperfine log that can also be found on the README. I chose the same candidates as Tealdeer to compare against.
My best guess is a large bottleneck that Tealdeer has is it's dynamic linking. I'm guessing this because I used to link against libcurl in my own client, and this added ~2ms to it's runtime. Here's what Tealdeer was linking against in it's release build as of 0716808
Replacing libcurl with a pure Zig implementation so that I could build as a static executable removed this 2ms and brought my average time back down to 0.1ms again. I'm sure that Tealdeer could benefit from this as well, by using pure Rust dependencies. Anyway, I'd appreciate it if you added my client to your benchmarks section. If you haven't used Zig before and need help building the source, feel free to reach out. |
As a further comment on speed, I agree that dynamic linking is another one, though I'd wager that the major difference is how Rust does printing compared to Zig and others, if you google
And this was done on a busy laptop, there are a few other things that I think might be a problem (Docopt being one of them). diff --git a/src/formatter.rs b/src/formatter.rs
index e2553c4..5095a8a 100644
--- a/src/formatter.rs
+++ b/src/formatter.rs
@@ -1,6 +1,6 @@
//! Functions related to formatting and printing lines from a `Tokenizer`.
-use std::io::BufRead;
+use std::io::{BufRead, Write};
use ansi_term::{ANSIString, ANSIStrings};
use log::debug;
@@ -68,6 +68,8 @@ where
R: BufRead,
{
let mut command = String::new();
+ let mut stdout = std::io::stdout();
+ stdout.lock();
while let Some(token) = tokenizer.next_token() {
match token {
LineType::Empty => {
@@ -83,13 +85,18 @@ where
command = title;
debug!("Detected command name: {}", &command);
}
- LineType::Description(text) => println!(" {}", config.style.description.paint(text)),
- LineType::ExampleText(text) => println!(" {}", config.style.example_text.paint(text)),
+ LineType::Description(text) => {
+ writeln!(stdout, " {}", config.style.description.paint(text)).unwrap()
+ }
+ LineType::ExampleText(text) => {
+ writeln!(stdout, " {}", config.style.example_text.paint(text)).unwrap()
+ }
LineType::ExampleCode(text) => {
- println!(" {}", &format_code(&command, &text, &config))
+ writeln!(stdout, " {}", &format_code(&command, &text, &config)).unwrap()
}
LineType::Other(text) => debug!("Unknown line type: {:?}", text),
}
}
- println!();
+ writeln!(stdout).unwrap();
+ stdout.flush().unwrap();
} |
@ve-nt Thank you for your input! The following PR looks very promising to me The benchmarks in the readme are out of date, we should maybe remove them 🤔 I have been fiddling around with benchmarking in Docker in another PR, but I am not happy with it yet. We will make sure to inspect your Zig client and include it if it is feasible to implement |
Docopt will definitely be removed in the future (see #108). |
@ve-nt You will have to help me out a bit if you want us to include outfieldr. I failed to build the client with the latest stable release (0.7). I saw that you mention 0.8-dev on your README, so I tried with the master branch of the language (the base image I wanted to use doesn't have a 0.8 version tagged), but the build fails here aswell. This is what the relevant part of the Dockerfile looks like: FROM euantorano/zig:master AS zig-builder
WORKDIR /build-outputs
RUN apk add git \
&& git clone https://gitlab.com/ve-nt/outfieldr.git \
&& cd outfieldr \
&& git submodule init \
&& git submodule update \
&& zig build -Drelease-safe |
@niklasmohrin One of the submodules needed to be updated after recent change in the standard library. I've done this and pushed, it should compile on |
I don't know whether you want to build in release-safe or release-fast for the benchmarks. I benchmarked against release-fast, however I expect that users of the program would want to use a release-safe build. The difference in performance between the two builds were pretty minor from my measurements. |
Yeah, I just used release-safe, see the PR if you want to review the benchmark |
It makes sense to use the build variant that would be used by packagers as well. I assume it's release-safe. |
https://github.com/sharkdp/hyperfine
As suggested in #38 (comment)
The text was updated successfully, but these errors were encountered: