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

Export dds_writecdr_impl #1880

Merged
merged 1 commit into from
Jan 30, 2024

Conversation

ymski
Copy link
Contributor

@ymski ymski commented Nov 16, 2023

Background

Hi, I am one of the development members of CARET.
Thanks for exporting dds_write_impl that were hooked in CARET in Issue 1308.
After having the symbol exported, CARET is able to perform the performance analysis as before.

We are currently developing the capability to analyze communications using GenericPublsiher.
To implement this feature, we need to hook dds_writecdr_impl, but as you know, this symbol is currently hidden.

Changes

Export dds_writecdr_impl symbol.
dds_writecdr_impl is a convenient trace point for instrumenting the code for performance measurements.

Related Links

@ymski ymski force-pushed the export-dds-writecdr-impl branch 2 times, most recently from fcf9e22 to e14440c Compare November 16, 2023 11:37
@ymski ymski marked this pull request as ready for review November 20, 2023 09:00
Signed-off-by: Koudai Yamasaki <[email protected]>
@ymski ymski force-pushed the export-dds-writecdr-impl branch from e14440c to b708d7a Compare December 15, 2023 01:53
@ymski
Copy link
Contributor Author

ymski commented Dec 25, 2023

@eboasson
May I request a review or reviewer assignment for this pull request?
Please contact me if it is incomplete.

@nabetetsu
Copy link

@eboasson
It is not something that requires an urgent response, but we would appreciate it if you could take the time in your busy shecule to review this PR.

Copy link
Contributor

@eboasson eboasson left a comment

Choose a reason for hiding this comment

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

@nabetetsu thanks for bumping this! @ymski, my apologies for not failing to really catch up after the holidays ...

I'd hoped to avoid having to export this one, but ... clearly this is an exported internal function and I won't promise not to break CARET in the future by changing this, but I am happy to promise that Cyclone will retain practical entry points for this type of instrumentation.

@eboasson eboasson merged commit 1c9bc19 into eclipse-cyclonedds:master Jan 30, 2024
21 checks passed
@nabetetsu
Copy link

@eboasson We greatly appreciate your kind consideration!

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

Successfully merging this pull request may close these issues.

3 participants