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

No stdout flush in console PresentMon 2.0. Is it intended? #232

Closed
AlexUnwinder opened this issue Apr 16, 2024 · 7 comments · Fixed by #338
Closed

No stdout flush in console PresentMon 2.0. Is it intended? #232

AlexUnwinder opened this issue Apr 16, 2024 · 7 comments · Fixed by #338
Assignees
Labels

Comments

@AlexUnwinder
Copy link

Noticed issues with much increased data lag in scenarios when I try to read data via stdout and console PresentMon-2.0.0-x64.exe comparing to the previous console versions. I started investigating it and nailed it down to fflush(stdout), which is performed inside UpdateCsv in V1.x but missing in V2.0. Was it intended, or flushing was simply lost during refactoring? If so, can it be returned back please?

In meanwhile I solved it by building a custom version PresentMon-2.0.0-x64.exe and adding the following to the very end of UpdateCsvT:

if (args.mCSVOutput == CSVOutput::Stdout)
    fflush(stdout);
@AlexUnwinder AlexUnwinder changed the title No stdout flush in console PresentMon 2.0. Is in intended? No stdout flush in console PresentMon 2.0. Is it intended? Apr 16, 2024
@JeffersonMontgomery-Intel
Copy link
Collaborator

Thanks, this wasn't intended I'll add it back in.

@AlexUnwinder
Copy link
Author

Excellent, thanks!

JeffersonMontgomery-Intel added a commit that referenced this issue Apr 17, 2024
This was accidentally removed 2.0.0, causing data lag issues for tools
using PresentMon console application with stdout output.

See issue #232
@AlexUnwinder
Copy link
Author

The issue was listed as fixed in 2.0.1, but it looks like it was not included in this version.

@JeffersonMontgomery-Intel
Copy link
Collaborator

@JeffersonMontgomery-Intel
Copy link
Collaborator

Never mind, it's missing for non --v1_metrics metrics for some reason :( https://github.com/GameTechDev/PresentMon/blob/v2.0.1/PresentMon/CsvOutput.cpp#L326

Sorry about that, not sure how it happened, will fix asap.

JeffersonMontgomery-Intel added a commit that referenced this issue May 8, 2024
Flushes were accidentally removed at some point.  v2.0.1 added them back
in, but only for the --use_v1_metrics CSV.  This change adds them back
in for the default CSV as well.

See issue #232
@AlexUnwinder
Copy link
Author

Sorry for bumping. Tired 2.1.0 but it seems to be missing there.

@AlexUnwinder AlexUnwinder reopened this Jul 25, 2024
@AlexUnwinder
Copy link
Author

Just peeked inside https://github.com/GameTechDev/PresentMon/archive/refs/tags/v2.1.0.zip and confirmed that fflush inside CsvOutput.cpp is still called for V1 metrics only. Please don't forget to include fflush for 2.0 metrics too in future versions.

@markgalvan-intel markgalvan-intel self-assigned this Aug 7, 2024
markgalvan-intel added a commit that referenced this issue Aug 7, 2024
Flushes were accidentally removed at some point.  v2.0.1 added them back in, but only for the --use_v1_metrics CSV.  This change adds them back in for the default CSV as well.

See issue #232
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants