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

New plot script: Run duration over time #1760

Merged
merged 6 commits into from Jul 30, 2019
Merged

New plot script: Run duration over time #1760

merged 6 commits into from Jul 30, 2019

Conversation

mrks
Copy link
Member

@mrks mrks commented Jul 22, 2019

Adds a script that plots the duration of individual benchmark runs over time. Example:

tpcc

Detailed version:

tpcc_detailed

@mrks mrks closed this Jul 22, 2019
@mrks mrks reopened this Jul 22, 2019
scripts/plot_run_duration_over_time.json Outdated Show resolved Hide resolved
with open(sys.argv[1]) as json_file:
data_json = json.load(json_file)['benchmarks']

# Build flat list of {name, begin, duration} entries for every benchmark item run
Copy link
Member Author

Choose a reason for hiding this comment

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

As is evident, I am not the biggest Python hacker. I can make things work, but there are probably various things that could be improved here. Please let me know!

@Bensk1
Copy link
Collaborator

Bensk1 commented Jul 22, 2019

  1. Why do new-order-prodcedures at the same point of time take from roughly 0.3 to roughly 1, do they add a different number of orders?
  2. Does it make sense to draw everything into the same graph? I cannot really see anything for payment, stock-level, and order-status.

@mrks
Copy link
Member Author

mrks commented Jul 23, 2019

Why do new-order-prodcedures at the same point of time take from roughly 0.3 to roughly 1, do they add a different number of orders?

They add 5 to 15 order lines, which is why you should see 11 "lines" in there. Bear in mind that everything is still super inefficient.

Does it make sense to draw everything into the same graph? I cannot really see anything for payment, stock-level, and order-status.

Added.

@Bensk1
Copy link
Collaborator

Bensk1 commented Jul 23, 2019

That looks already much better. If we create a collection of these over time, I am sure we can sell it as modern art at some point.

@mrks
Copy link
Member Author

mrks commented Jul 24, 2019

Anything else that needs to be done here?

Bensk1
Bensk1 previously approved these changes Jul 25, 2019
@mrks mrks added the FullCI Run all CI tests (slow, but required for merge) label Jul 29, 2019
@mrks
Copy link
Member Author

mrks commented Jul 29, 2019

Closing this for now as #1766 will change the JSON format. Will re-open and change to new JSON format once that is done.

@mrks mrks closed this Jul 29, 2019
@mrks mrks reopened this Jul 29, 2019
@mrks mrks requested a review from Bensk1 July 29, 2019 14:15
@mrks mrks dismissed Bensk1’s stale review July 29, 2019 14:15

Please re-review the changes made in 12c7bcc

data.append({'name': name, 'begin': begin / 1e9, 'duration': duration})
for run_json in benchmark_json['successful_runs']:
data.append({'name': name, 'begin': run_json['begin'] / 1e9, 'duration': run_json['duration'], 'success': True})
for run_json in benchmark_json['unsuccessful_runs']:
Copy link
Member Author

Choose a reason for hiding this comment

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

I'm not really happy with the code duplication here, but I have no idea how to avoid it without making everything more verbose.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I am sure we can handle these two duplicated lines

@mrks mrks merged commit 22d86ec into master Jul 30, 2019
@mrks mrks deleted the mrks/run_duration branch July 30, 2019 08:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
FullCI Run all CI tests (slow, but required for merge)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants