Skip to content
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

[draft/wip] Adding (initial) fortio.org/log entry #1317

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

ldemailly
Copy link

@ldemailly ldemailly commented Aug 8, 2023

Ran make updatereadme and go test -v -bench="." -benchtime=3s -benchmem on my M1 max laptop

Using this to improve and compare fortio's logger perf

@CLAassistant
Copy link

CLAassistant commented Aug 8, 2023

CLA assistant check
All committers have signed the CLA.

Copy link
Collaborator

@abhinav abhinav left a comment

Choose a reason for hiding this comment

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

I'm guessing this is still WIP.
I'm supportive of adding another entry to the list when you're ready.

benchmarks/go.mod Outdated Show resolved Hide resolved
Comment on lines +38 to +39
// Go optimizes away allocations for numbers below 256
_tenInts = []int{1001, 1002, 1003, 1004, 1005, 1006, 1007, 1008, 1009, 1010}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Fair point. If you're still experimenting with this PR, you can probably get this specific change merged as a separate PR.

Copy link
Author

Choose a reason for hiding this comment

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

thanks btw, how / on which machine do you run make updatereadme as the absolute (and seemingly even some times the relative numbers) depends on the hw/arch/os it's being ran on ?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think original runs were on Intel Macbook Pros, and since then on Apple Silicon Macbook Pros.
Absolute numbers are less important than the relative performance, so as long as the numbers are consistent and the benchmarks were all run on the same machine, it should be good to check-in.

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

Successfully merging this pull request may close these issues.

4 participants