Skip to content

Optimize CSV monitor output for 2x runtime#412

Open
lukelowry wants to merge 2 commits into
developfrom
lukel/perf-csv-dev
Open

Optimize CSV monitor output for 2x runtime#412
lukelowry wants to merge 2 commits into
developfrom
lukel/perf-csv-dev

Conversation

@lukelowry
Copy link
Copy Markdown
Collaborator

Description

This stacked PR optimizes CSV monitor output in PhasorDynamics by formatting CSV values directly into a reusable line buffer in the hot monitor path. It is stacked on #411 (lukel/perf-sys-dev).

Proposed changes

  • Add protected CSV append hooks to VariableMonitorBase with existing stream-print fallbacks.
  • Add direct CSV append support for scalar monitor values using std::to_chars for floating-point output.
  • Reuse a single CSV line buffer in VariableMonitorController and write each CSV row once.
  • Leave JSON/YAML output and direct CSV stream-print methods on their existing stream behavior.

Performance was measured with application/PhasorDynamics/PDSim using the app-reported Complete in time. Each row reports the median of 3 trials after rebuilding PDSim on the corresponding branch.

Case lukel/perf-sys-dev (s) This branch (s) Improvement
Hawaii 0.172773 0.124430 28.0%
NewEngland 0.229073 0.169727 25.9%
Illinois 0.652370 0.319652 51.0%
WECC 1.153270 0.576705 50.0%
Texas 8.437620 4.311520 48.9%

Checklist

  • All tests pass. (except the one PE failure)
  • Code compiles cleanly with flags -Wall -Wpedantic -Wconversion -Wextra.
  • The new code follows GridKit(TM) style guidelines.
  • N/A: no new unit tests for this narrow CSV output fast path.
  • N/A: no user-facing documentation change.
  • The feature branch is based on the target branch.
  • N/A: no CHANGELOG.md update is needed for this internal PhasorDynamics monitor-output optimization.

Further comments

stoked

@lukelowry lukelowry requested review from PhilipFackler and pelesh May 22, 2026 06:09
@lukelowry lukelowry added the enhancement New feature or request label May 22, 2026
Copy link
Copy Markdown
Collaborator

@PhilipFackler PhilipFackler left a comment

Choose a reason for hiding this comment

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

This approach is great! Nice results. Thanks for spending time on this. Looks like I trusted the std::ostream buffer too much. 😸

I would like you to revert the structure a bit, though.

I think you should use this approach for all of the formats. No need to have extra custom CSV functions and separate bits in the ValuePrinter. Use the string buffer in the controller (call it something generic like buffer_ instead of csv_line_) and change the print* functions internally to use the interface you've made here for appendCsv. You could change them to append also, to make it clearer (don't need two different implementations for the same thing). You shouldn't have to bother much with the implementations for the other formats (just switch to using stringstream).

In other words, just replace my implementation with yours everywhere. 😆

How does that sit with you?

@PhilipFackler
Copy link
Copy Markdown
Collaborator

Also, shouldn't this be aimed at develop?

@lukelowry
Copy link
Copy Markdown
Collaborator Author

Also, shouldn't this be aimed at develop?

I can change the target branch, I only kept it like this because I measured it against my other branch, not this one! I will shift target so it's async with the other PR.

Good on unifying across the three formats, I just don't know much about yaml. Otherwise as long as you and @pelesh are okay with this, I can certainly extend this to the other formats and make api clearer. Thank you for the feedback!

@PhilipFackler
Copy link
Copy Markdown
Collaborator

I just don't know much about yaml

They all use std::ostream, so you just need to change it to append everything to the buffer instead of inserting to the stream. Then the top level should work the same as you have in your CSV version.

@pelesh pelesh changed the base branch from lukel/perf-sys-dev to develop May 22, 2026 18:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants