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

Add repeat option to CLI to run process N times #3616

Merged
merged 1 commit into from
Oct 26, 2020
Merged

Conversation

jelovirt
Copy link
Member

@jelovirt jelovirt commented Oct 25, 2020

Description

Add repeat option to CLI to run conversion multiple times.

The output for repeated conversions is output to standard out after all conversions have been run, e.g.

1 17531ms
2 9199ms
3 8077ms
4 7832ms
5 7693ms

Because this is a measurement tool, the output should not be part of the log output.

Motivation and Context

This option is useful for performance measurement, since timings for the first transformation are often dominated by Java warm-up time.

How Has This Been Tested?

Old tests pass.

Type of Changes

  • New feature (non-breaking change which adds functionality)

Documentation and Compatibility

Option is documented in the CLI help, but docs could mention that this option is intended for plug-in developers for measuring performance.

Signed-off-by: Jarno Elovirta <jarno@elovirta.com>
@jelovirt jelovirt self-assigned this Oct 25, 2020
@jelovirt jelovirt added CLI Command-line interface issues feature New feature or request priority/low Low priority issue labels Oct 25, 2020
@jelovirt jelovirt added this to In progress in Next via automation Oct 25, 2020
@jelovirt jelovirt moved this from In progress to Review in progress in Next Oct 25, 2020
@jelovirt
Copy link
Member Author

jelovirt commented Oct 25, 2020

Do you think we should improve this by calculating the average time and outputting it as the last time? Because of the JVM warm-up, we'd need to ignore the warm-up phase from the average, so the option would need to be something like --repeat=5,10 where the 5 is the number of warm-up conversions and ´10´ is how many actual conversions are run. The average would then be calculated from the 10 conversions. Or do we just leave this to the user and only output the raw data?

🤔

Better to leave the analytics to the users and just output the raw data.

@jelovirt
Copy link
Member Author

jelovirt commented Oct 25, 2020

Or should be output be

Processing time in ms:
17531
9199
8077
7832
7693

@raducoravu
Copy link
Member

@jelovirt maybe you can output both the individual times and the average.

Next automation moved this from Review in progress to Reviewer approved Oct 26, 2020
@jelovirt jelovirt merged commit aaea8fc into develop Oct 26, 2020
Next automation moved this from Reviewer approved to Done Oct 26, 2020
@jelovirt jelovirt deleted the feature/cli-repeat branch October 26, 2020 18:54
@jelovirt jelovirt added this to the Next milestone Oct 26, 2020
@robander
Copy link
Member

It does seem that average would not mean a lot if it includes the first. Also (based on trying similar things today), it messes up when I've got 15 builds running and hand to go use Slack during one, which increased that one build time by 20%...

infotexture added a commit to dita-ot/docs that referenced this pull request Dec 6, 2020
Per dita-ot/dita-ot#3616

Signed-off-by: Roger Sheen <roger@infotexture.net>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLI Command-line interface issues feature New feature or request priority/low Low priority issue
Projects
No open projects
Next
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

3 participants