Skip to content

[benchmark] run_smoke_bench tweaks #20667

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

Merged
merged 2 commits into from
Nov 26, 2018

Conversation

palimondo
Copy link
Contributor

@palimondo palimondo commented Nov 17, 2018

Two small fixes:

  • use SWIFT_DETERMINISTIC_HASHING
  • allow running tests with dot and dash in the name

The naming change is to allow for measuring tests conforming to proposed naming convention in #20334 (only partial support for . and -, for now without ! and ?), like the #20552 and #20666.

If these get accepted, the Apple-internal script that measures benchmarks and submits them to LNT should also be updated to work with such names.

Tests that used hashing were being unnecessarily tested multiple times, because this environment variable was missing.
@palimondo
Copy link
Contributor Author

@eeckstein Please review 🙏

Allow for running of test matching the naming convention proposed in swiftlang#20334.
@eeckstein
Copy link
Contributor

@swift-ci benchmark

1 similar comment
@eeckstein
Copy link
Contributor

@swift-ci benchmark

@eeckstein eeckstein self-requested a review November 26, 2018 17:50
Copy link
Contributor

@eeckstein eeckstein left a comment

Choose a reason for hiding this comment

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

lgtm

@eeckstein
Copy link
Contributor

@swift-ci smoke test

1 similar comment
@eeckstein
Copy link
Contributor

@swift-ci smoke test

@swift-ci
Copy link
Contributor

Build comment file:

No performance and code size changes


@eeckstein eeckstein merged commit 5a6dfb6 into swiftlang:master Nov 26, 2018
@palimondo
Copy link
Contributor Author

Thank you Erik!

@palimondo
Copy link
Contributor Author

@eeckstein just quick a reminder from above, as it looks like #20769 also uses periods in names:

If these get accepted, the Apple-internal script that measures benchmarks and submits them to LNT should also be updated to work with such names.

@lorentey
Copy link
Member

If the dots cause any trouble, I can still change them!

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.

4 participants