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.

Sign up for GitHub

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

Jump to bottom

Add a stand-alone "timing" program (PR 15088) #16540

Closed
wants to merge 1 commit into from
Closed

Add a stand-alone "timing" program (PR 15088) #16540

wants to merge 1 commit into from

Conversation

richsalz
Copy link
Contributor

@richsalz richsalz commented Sep 7, 2021

This is a re-creation of #15088

@richsalz richsalz changed the title Add a stand-alone "timing" program (re-creation of #15088) Add a stand-alone "timing" program (PR #15088) Sep 7, 2021
@richsalz richsalz changed the title Add a stand-alone "timing" program (PR #15088) Add a stand-alone "timing" program (PR 15088) Sep 7, 2021
@t8m t8m added approval: otc review pending branch: master Merge to master branch triaged: feature The issue/pr requests/adds a feature labels Sep 8, 2021
@DDvO
Copy link
Contributor

DDvO commented Sep 8, 2021

This is a re-creation of #15088

I wonder why you re-recreated it?
This way it is harder to follow-up on the results you posted in #15088.

@t8m
Copy link
Member

t8m commented Sep 8, 2021

I wonder why you re-recreated it?

It was moved to a different fork repo.

@tmshort
Copy link
Contributor

tmshort commented Sep 9, 2021

@richsalz moved the source to the Akamai github repo from his personal repo.

@t8m t8m added this to the Post 3.0.0 milestone Sep 24, 2021
@beldmit beldmit mentioned this pull request Oct 8, 2021
Open
@openssl-machine
Copy link
Collaborator

This PR is in a state where it requires action by @openssl/otc but the last update was 186 days ago

@openssl-machine
Copy link
Collaborator

This PR is in a state where it requires action by @openssl/otc but the last update was 217 days ago

@openssl-machine
Copy link
Collaborator

This PR is in a state where it requires action by @openssl/otc but the last update was 248 days ago

@beldmit beldmit mentioned this pull request May 19, 2022
Closed
2 tasks
@openssl-machine
Copy link
Collaborator

This PR is in a state where it requires action by @openssl/otc but the last update was 279 days ago

@openssl-machine
Copy link
Collaborator

This PR is in a state where it requires action by @openssl/otc but the last update was 310 days ago

@DDvO
Copy link
Contributor

DDvO commented Jul 17, 2022

I wonder what the current state of affairs regarding inefficiency of credential loading is.
Is this still a problem? I fear so.
Are there open issue reports that can be linked to this tool?

@DDvO DDvO mentioned this pull request Jul 17, 2022
Open
@DDvO
Copy link
Contributor

DDvO commented Jul 17, 2022 •
edited
Loading

I've just filed #18814, which makes use of this tool.
This shows that there is still much inefficiency with credential loading.

@richsalz
Copy link
Contributor Author

Glad you find it useful.
My interest is in the steady-state overhead.

@paulidale
Copy link
Contributor

I've used this too, I think it should be merged.

@paulidale paulidale added approval: review pending This pull request needs review by a committer and removed approval: otc review pending labels Jul 18, 2022
slontis
slontis reviewed Jul 18, 2022
View reviewed changes
test/timing.c
* https://www.openssl.org/source/license.html
*
* I compiled this in the source tree this way:
* gcc -g -Iinclude a.c libcrypto.a -ldl -lpthread
Copy link
Member

Choose a reason for hiding this comment

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

a.c ?

Copy link
Contributor

Choose a reason for hiding this comment

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

Year, that header comment is outdated and should be removed.

As written in the OP of #18814, one currently can use

gcc -g -I../include timing.c ../libcrypto.a -ldl -lpthread -o timing

DDvO
DDvO requested changes Jul 18, 2022
View reviewed changes
Copy link
Contributor

@DDvO DDvO left a comment

Choose a reason for hiding this comment

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

For proper inclusion of this tool in test/, it should added to test/build.info,
and the two test credentials files should better be moved to test/certs/.

@richsalz
Copy link
Contributor Author

I am not contributing to OpenSSL, please take this PR and do what you want/need.

@DDvO DDvO mentioned this pull request Jul 18, 2022
Closed
@DDvO
Copy link
Contributor

DDvO commented Jul 18, 2022

I am not contributing to OpenSSL, please take this PR and do what you want/need.

Fixups are now #18821 - I had to copy this because I cannot write to the akamai repo.
So closing this one.

@DDvO DDvO closed this Jul 18, 2022
@tmshort
Copy link
Contributor

tmshort commented Jul 22, 2022

@DDvO, I can write to the Akamai repo...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@slontis slontis slontis left review comments

@DDvO DDvO DDvO requested changes

@paulidale paulidale paulidale approved these changes

Assignees
No one assigned
Labels
approval: review pending This pull request needs review by a committer branch: master Merge to master branch triaged: feature The issue/pr requests/adds a feature
Projects
None yet
Milestone
Post 3.0.0
Development

Successfully merging this pull request may close these issues.
7 participants
@richsalz @DDvO @t8m @tmshort @openssl-machine @paulidale @slontis