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 benchcat: cat + throughput measurements #15564

Merged
merged 7 commits into from
Jun 30, 2023
Merged

Add benchcat: cat + throughput measurements #15564

merged 7 commits into from
Jun 30, 2023

Conversation

lhecker
Copy link
Member

@lhecker lhecker commented Jun 16, 2023 •
edited
Loading

benchcat, "bc" for short, is a tool that I've written over the last
two years to help me benchmark OpenConsole and Windows Terminal.
Initially it only measured the time it took to print a file as fast as
possible, but it's grown to support a number of arguments, including
chunk (WriteFile call) sizes, repeat counts and VT mode with italic
and colorized output. In the future I also wish to add a way to
generate the output data on the fly via command line arguments.

One unusual trait of benchcat is that it is compiled entirely without
CRT and vcruntime. I did this so that I could test it on Windows XP.
Also, it's kind of funny seeing how it's only about 11kB.

This commit also fixes a couple $LASTEXITCODE cases, because our
spellchecker was bothering me a lot with this PR and so I just fixed it.

@lhecker lhecker force-pushed the dev/lhecker/benchcat branch from 0b9b597 to 0b1b099 Compare June 16, 2023 14:09
DHowett
DHowett approved these changes Jun 21, 2023
View reviewed changes
Copy link
Member

@DHowett DHowett left a comment

Choose a reason for hiding this comment

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

Outstanding.

src/tools/benchcat/main.cpp Outdated Show resolved Hide resolved
src/tools/benchcat/main.cpp

int __stdcall main() noexcept
{
SetConsoleOutputCP(CP_UTF8);
Copy link
Member

Choose a reason for hiding this comment

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

note: destroys current codepage

lhecker
lhecker commented Jun 22, 2023
View reviewed changes
src/tools/benchcat/main.cpp Outdated Show resolved Hide resolved
@lhecker
Copy link
Member Author

lhecker commented Jun 22, 2023 •
edited
Loading

BTW anyone got an idea what I did wrong with the project setup that I'm getting this?

src\tools\RenderingTests\main.cpp(0,0): Error C1041: cannot open program database 'C:\a\_work\1\s\obj\x64\Release\vc143.pdb'; if multiple CL.EXE write to the same .PDB file, please use /FS

Edit: Why does it compile RenderingTests anyways? ô_O

carlos-zamora
carlos-zamora reviewed Jun 22, 2023
View reviewed changes
OpenConsole.sln
Comment on lines +2816 to +2820
{2C836962-9543-4CE5-B834-D28E1F124B66}.Fuzzing|Any CPU.ActiveCfg = Fuzzing|Win32
{2C836962-9543-4CE5-B834-D28E1F124B66}.Fuzzing|ARM.ActiveCfg = Fuzzing|Win32
{2C836962-9543-4CE5-B834-D28E1F124B66}.Fuzzing|ARM64.ActiveCfg = Fuzzing|ARM64
{2C836962-9543-4CE5-B834-D28E1F124B66}.Fuzzing|x64.ActiveCfg = Fuzzing|x64
{2C836962-9543-4CE5-B834-D28E1F124B66}.Fuzzing|x86.ActiveCfg = Fuzzing|Win32
Copy link
Member

Choose a reason for hiding this comment

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

Probably don't need this for Fuzzing right?

OpenConsole.sln
Comment on lines +2803 to +2804
{2C836962-9543-4CE5-B834-D28E1F124B66}.AuditMode|Any CPU.ActiveCfg = AuditMode|Win32
{2C836962-9543-4CE5-B834-D28E1F124B66}.AuditMode|ARM.ActiveCfg = AuditMode|Win32
Copy link
Member

Choose a reason for hiding this comment

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

Do we want this for audit mode too?

Copy link
Member

Choose a reason for hiding this comment

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

We must include a configuration mapping for every platform|config combo in the solution.

Since these do not have a .0 entry (instead of .ActiveCfg), they will not be considered for build.

This and the above are in the correct state :)

@DHowett
Copy link
Member

DHowett commented Jun 22, 2023

BTW anyone got an idea what I did wrong with the project setup that I'm getting this?

I'll need to look. That's definitely ~ ~ strange ~ ~

@lhecker
Copy link
Member Author

lhecker commented Jun 23, 2023

I figured out why it didn't work: I forgot to set the <ProjectName> in the vcxproj file.
Also, I'm now using CommandLineToArgvW which makes the code quite a bit more compact. I've also restructured the PR code a little bit to restore the console mode and codepage properly on exit.

@lhecker
Copy link
Member Author

lhecker commented Jun 26, 2023

@DHowett I've removed all of the /src/tools projects from building by default when you build the .sln. Are you fine with that?

@DHowett
Copy link
Member

DHowett commented Jun 26, 2023

Are you fine with that?

That seems like too broad of a change for this PR that just adds a new project. Why?

Also, some of the tools are required for the feature tests. 😄

Summary: Total=388, Passed=10, Failed=378, Blocked=0, Not Run=0, Skipped=0

@lhecker
Copy link
Member Author

lhecker commented Jun 26, 2023

That seems like too broad of a change for this PR that just adds a new project. Why?

I'll revert it. Generally, I think it'd be better if we didn't build non-essential projects by default.

carlos-zamora
carlos-zamora approved these changes Jun 29, 2023
View reviewed changes
Copy link
Member

@carlos-zamora carlos-zamora left a comment

Choose a reason for hiding this comment

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

More tools are nice. Thanks!

@lhecker lhecker added the AutoMerge Marked for automatic merge by the bot when requirements are met label Jun 30, 2023
@microsoft-github-policy-service microsoft-github-policy-service bot enabled auto-merge (squash) June 30, 2023 13:44
@microsoft-github-policy-service microsoft-github-policy-service bot deleted the dev/lhecker/benchcat branch June 30, 2023 14:18
DHowett
DHowett reviewed Jun 30, 2023
View reviewed changes
OpenConsole.sln
@@ -2793,11 +2792,28 @@ Global
{37C995E0-2349-4154-8E77-4A52C0C7F46D}.Release|Any CPU.ActiveCfg = Release|Win32
{37C995E0-2349-4154-8E77-4A52C0C7F46D}.Release|ARM.ActiveCfg = Release|Win32
{37C995E0-2349-4154-8E77-4A52C0C7F46D}.Release|ARM64.ActiveCfg = Release|ARM64
{37C995E0-2349-4154-8E77-4A52C0C7F46D}.Release|ARM64.Build.0 = Release|ARM64
Copy link
Member

Choose a reason for hiding this comment

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

removing RenderingTests from the build?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, intentionally. I never intended that code to build in the CI tbh. I just needed it during development and maybe only once or twice per year from now on. 😅

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

@DHowett DHowett DHowett approved these changes

@carlos-zamora carlos-zamora carlos-zamora approved these changes

Assignees
No one assigned
Labels
AutoMerge Marked for automatic merge by the bot when requirements are met
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@lhecker @DHowett @carlos-zamora