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

aggregate template instantiations #39

Closed
Trass3r opened this issue Oct 10, 2022 · 10 comments
Closed

aggregate template instantiations #39

Trass3r opened this issue Oct 10, 2022 · 10 comments
Labels
enhancement New feature or request
Projects

Comments

@Trass3r
Copy link
Contributor

Trass3r commented Oct 10, 2022

Just looked at Instance Function and there were many individual entries of the form class<T>::method<U>.
It would be nice to have the option to aggregate them by method, by class or both (displayed like class<...>::method<...>).

@Viladoman Viladoman added the enhancement New feature or request label Oct 13, 2022
@Viladoman
Copy link
Owner

Sounds good! That's a really good idea.

It think this might need to be an option for the data extractor itself, as I can't properly merge on the extension the accumulated times when dealing with nested initializations and such. As you don't want a child instantiation to count double time. Also collapsing template arguments on the class level will mean that nested structures will all have its template arguments collapsed.

This should be a simple string replace when reading the symbols.

@Viladoman Viladoman added this to Open in Extractor Oct 13, 2022
@Trass3r
Copy link
Contributor Author

Trass3r commented Oct 13, 2022

I also saw that ClangBuildAnalyzer does something similar, using <$> notation, e.g.:
https://github.com/Trass3r/llvm-project/actions/runs/3479782039/jobs/5818745618#step:6:163

1080 ms: std::tuple_element<$>::type& std::get<$>(std::tuple<$>&) (145 times, avg 7 ms)

@Trass3r
Copy link
Contributor Author

Trass3r commented Feb 6, 2023

And WPA: https://devblogs.microsoft.com/cppblog/profiling-template-metaprograms-with-cpp-build-insights/
Thinking about it, maybe CompileScore could offer to open the .etl file that probably gets generated anyway in WPA for advanced analysis.
Then I could save that manual step:

vcperf -start -level3 MyVCSession
vcperf -stop -templates MyVCSession build.etl
wpa build.etl

@Viladoman
Copy link
Owner

I am not sure I follow the question here. Are we talking about the CompileScore App or Visual Studio Extension?

You can perform the same .etl generation that vcperf does using the ScoreDataExtractor. This way you can build the project once, extract the .etl/.ctl and then extract different .scor ( aggregation of data ) with different parameters and sizes.

When performing the stop and generating the scor directly, the .etl generation is skipped as it is not needed and we can instead parse the events directly and faster.

tldr: the ScoreDataExtractor can generate:

  • build -> scor
  • build -> etl/ctl
  • etl/ctl -> scor

Technically if we want to allow the App to process .etl directly, we can just make the App perform :
ScoreDataExtractor.exe -extract -msvc -i Provided.etl -o TempScore.scor. And load that temporary file.

Tbh, the more flexible the input format is the better, it could even be a folder with the .json files that clang produces too.

Is this what you meant?

@Trass3r
Copy link
Contributor Author

Trass3r commented Feb 7, 2023

The Visual Studio extension.
I thought there would be a temporary .etl anyway that can be offered to open in WPA.

@Viladoman
Copy link
Owner

I understand, Adding an option in the extension options to also generate the etl when recording the events should pretty simple to add.

If I may ask, for what features do you need to use WPA?

@Trass3r
Copy link
Contributor Author

Trass3r commented Feb 8, 2023

Well it was inspired by this topic, aggregating template instantiations.
But of course in general WPA allows advanced usage like defining your own columns and grouping by select columns.
They have an interesting article series about what you can do:
https://learn.microsoft.com/en-us/cpp/build-insights/get-started-with-cpp-build-insights#articles
Includes a feature to estimate wall clock time impact in addition to just simply adding up the times.
And it's really useful to see average, min and max too (though I'd prefer median and deviation):
Neargye/magic_enum#239 (comment)

@Viladoman Viladoman moved this from Open to In progress in Extractor Feb 9, 2023
@Viladoman Viladoman moved this from In progress to Done in Extractor Feb 28, 2023
@Viladoman
Copy link
Owner

Closing this issue as the template arguments can now be collapsed on version 1.8.1 (enabled by default).

@Trass3r
Copy link
Contributor Author

Trass3r commented Mar 15, 2023

Thanks!
Should we create a separate ticket for

Adding an option in the extension options to also generate the etl when recording the events should pretty simple to add.

@Viladoman
Copy link
Owner

Yes please.

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
Extractor
  
Done
Development

No branches or pull requests

2 participants