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

- run-perf.pl initial commit #3

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Conversation

Sashan
Copy link
Contributor

@Sashan Sashan commented Jul 16, 2024

script is able to run perf tools and collect data in .markdown format on windows, unix (BSD), macos and linux. It is perl variant of the run-perl.sh (same tool implemented in bash). Unlike bash variant this version requires tools to be compiled first.

There seems to be no easy way to run bash script on windows. the perl seems to be better bet.

@t8m t8m added triaged: feature The issue/pr requests/adds a feature approval: review pending This pull request needs review by a committer approval: otc review pending This pull request needs review by an OTC member labels Jul 17, 2024
@Sashan Sashan force-pushed the run-perf.pl branch 3 times, most recently from cac68df to e698770 Compare July 19, 2024 06:45
@Sashan
Copy link
Contributor Author

Sashan commented Jul 19, 2024

The list of force pushed changes since initial commit:

  • rwlocks tool returns two numbers, the first is stat for write lock, second is read lock stats, I got it wrong first
  • rwlocks tool when running for single thread returns NaN for write lock stats, the NaN result is handled now
  • tool reports a test name and openssl version it is currently testing

@Sashan
Copy link
Contributor Author

Sashan commented Aug 21, 2024

new revision also supports stuff from @andrewkdinh which got mergerd recently (#4)

@t8m
Copy link
Member

t8m commented Aug 21, 2024

Could you please drop the commits from the Andrew's PR? They are already merged.

@Sashan
Copy link
Contributor Author

Sashan commented Aug 21, 2024

I think I will start with fresh pull-request. there must be something wrong with my clone
will figure it out later.

poor git driving -- again.

@Sashan
Copy link
Contributor Author

Sashan commented Aug 21, 2024

Now the PR is in shape I'd like to merge in.
thanks for patience.

@t8m t8m removed the approval: otc review pending This pull request needs review by an OTC member label Aug 29, 2024
@t8m
Copy link
Member

t8m commented Aug 29, 2024

ping @levitte for second review

@Sashan
Copy link
Contributor Author

Sashan commented Aug 29, 2024

ping @levitte for second review

please hold a sec, I need to update this change with stuff which has currently arrived
vi PR#6 and PR#12. Will do it right now.

@Sashan
Copy link
Contributor Author

Sashan commented Aug 29, 2024

done. new version

in .markdown format on windows, unix (BSD), macos
and linux. It is perl variant of the run-perl.sh
(same tool implemented in bash). Unlike bash variant
this version requires tools to be compiled first.

There seems to be no easy way to run bash script
on windows. the perl seems to be better bet.
run-perf.pl Show resolved Hide resolved
run-perf.pl Outdated Show resolved Hide resolved
Co-authored-by: Viktor Dukhovni <viktor1ghub@dukhovni.org>
@t8m
Copy link
Member

t8m commented Aug 30, 2024

Another option would be to just skip rwlock measurements for now. IMO they are not particularly useful anyway as they just measure the speed of something that is not going to change much. In this sense the benchmark is completely synthetic and outside of OpenSSL realm. It was useful for comparing the rwlock performance with rcu locks but not useful that much anymore.

@t8m t8m added approval: done This pull request has the required number of approvals and removed approval: review pending This pull request needs review by a committer labels Aug 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approval: done This pull request has the required number of approvals triaged: feature The issue/pr requests/adds a feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants