Skip to content

Conversation

@ywkaras
Copy link
Contributor

@ywkaras ywkaras commented May 23, 2023

Eliminate TSDbg(), TSIsDbgCtlSet(), TSDbgCtlCreate(), TSDbgCtlDestroy().

Eliminate TSDbgCtlUniqPtr type.

@ywkaras ywkaras requested a review from zwoop May 23, 2023 18:51
@ywkaras ywkaras self-assigned this May 23, 2023
@ywkaras ywkaras added the Debug Support for system debugging label May 23, 2023
@ywkaras ywkaras added this to the 10.0.0 milestone May 23, 2023
@ywkaras ywkaras requested a review from SolidWallOfCode May 23, 2023 20:29
@ywkaras
Copy link
Contributor Author

ywkaras commented May 23, 2023

Doesn't yet have all the doc changes that would be necessary.


friend void TSDbgCtlDestroy(TSDbgCtl const *dbg_ctl);
};
using DbgCtl = TSDbgCtl;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe just absorb this header into DiagsTypes.h

@ywkaras
Copy link
Contributor Author

ywkaras commented May 25, 2023

After thinking about it some, I realize it would cause problems for Yahoo (and perhaps others) to change TSDbgCtl to a class. I'd prefer to add the namespace tsapi for C++ stuff in the TS API. So we'd add the class DbgCtl and the function Dbg() to this namespace. We could also have a member function operator for DbgCtl to optionally shorted debug output calls a bit, I think.

@ywkaras ywkaras force-pushed the CppDbgCtl branch 2 times, most recently from 806ab86 to 5c8af80 Compare August 24, 2023 14:14
Eliminate TSDbg(), TSIsDbgCtlSet(), TSDbgCtlCreate(), TSDbgCtlDestroy().

Eliminate TSDbgCtlUniqPtr type.
@ywkaras ywkaras changed the title Change TSDbgCtl from a struct to a class. Use Dbg() for debug output in both core and plugins. Aug 24, 2023
@ywkaras
Copy link
Contributor Author

ywkaras commented Aug 24, 2023

This sets the stage for removing and replacing slow TSDebug() calls.

@ywkaras
Copy link
Contributor Author

ywkaras commented Aug 24, 2023

I will make the doc changes for this in a follow on PR that also removes TSDebug().

@ywkaras ywkaras marked this pull request as ready for review August 24, 2023 18:32
if (!m) {
return false;
}
if (!_ptr->second) {
Copy link
Contributor Author

@ywkaras ywkaras Aug 24, 2023

Choose a reason for hiding this comment

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

Technically second should be std::atomic (or at least volatile), since it can be written from another thread. But doing that risks causing the generation of less performant code. And, it seems unlikely that any register buffering of the value would cause actual problems. std::atomic instances are not moveable, so they can' t be used in STL containers.

@ywkaras
Copy link
Contributor Author

ywkaras commented Aug 28, 2023

[approve ci]

Copy link
Contributor

@zwoop zwoop left a comment

Choose a reason for hiding this comment

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

Love it. We still have work to do in many plugins though, right?

@ywkaras ywkaras merged commit 0f4ee82 into apache:master Aug 29, 2023
@ywkaras
Copy link
Contributor Author

ywkaras commented Aug 29, 2023

Love it. We still have work to do in many plugins though, right?

Yes

cmcfarlen pushed a commit to cmcfarlen/trafficserver that referenced this pull request Jun 3, 2024
* asf/master:
  Make bad disk detection more robust (apache#10317)
  Update Proxy Verifier to v2.10.1 (apache#10322)
  Fix lua plugin build (apache#10319)
  cmake: Add support for building benchmarks (apache#10316)
  cmake: add tests for proxy/http3 (apache#10310)
  Cmake: Add tests in proxy/http proxy/http2 (apache#10305)
  add tests for src/records (apache#10302)
  cmake: add unit tests for proxy/logging (apache#10301)
  setup default install path, runtime user and group (apache#10299)
  cmake: add tests for proxy/hdrs (apache#10283)
  Fix ip_allow optional methods specification (apache#10246)
  Plugin promotions, deprecations and deletions (apache#10303)
  Use Dbg() for debug output in both core and plugins. (apache#9732)
  Remove in_addr forward declaration from experimental.h. (apache#10309)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Debug Support for system debugging

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

2 participants