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

[core] New formatting implementation using SFMT for the logging system #2955

Draft
wants to merge 22 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from 16 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
be1e774
[core] New implementation for the logging/formatting system
ethouris May 24, 2024
308af58
Fixed parts in non-default-enabled code parts
ethouris May 24, 2024
5ae2a03
Updated sfmt.h with reimplementation fixes
ethouris May 25, 2024
089dd51
Fixed heavy logging cases and atomics. Withdrawn changes for enums. U…
ethouris May 25, 2024
e37d37d
Provided a special version for atomic. Withdrawn unnecessary changes
ethouris May 26, 2024
6ce84fe
Changed more formatting usage to sfmt
ethouris May 26, 2024
8126c49
Removed ostringstream from utilities
ethouris May 26, 2024
216c1ed
Removed ostringstream use. Fixed C++03 problem with Ensure declaration
May 27, 2024
d8cebcd
Cleared out warn-errors for logging-off version
May 27, 2024
4e39fd7
Moved Printable out of C++11-dependent section (args... no longer nee…
May 27, 2024
5199746
Added extra version of snprintf for old Windows
ethouris May 27, 2024
4843143
Fixed the use of std::atomic
ethouris May 28, 2024
1a0eca4
Fixed the right atomic type used with logging. Fixed some reported sh…
ethouris May 28, 2024
9643702
Fixed a clang-reported warning (to trigger rebuilding)
ethouris May 28, 2024
f4ecbc1
Updated sfmt.h, fixed sync formatting
Jun 17, 2024
b90f540
Updated and merged
Jun 17, 2024
7660e92
Merge branch 'master' into dev-add-sfmt-for-logging
Jun 18, 2024
0a607c7
Some cosmetic fixes. Fixed the use of std::abs
Jun 18, 2024
d2ec1cf
Fixed usage of <cmath> with std
Jun 19, 2024
193fe39
Fixed correct includes for std::div
Jun 19, 2024
c81d4d6
Renamed sfmt.h and moved to srt namespace
Jun 20, 2024
fcf93cf
Fixed: sfmt should use only void* pointers. Fixed: std::hex should no…
Jun 24, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion apps/logsupport.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,7 @@ set<srt_logging::LogFA> SrtParseLogFA(string fa, set<string>* punknown)

void ParseLogFASpec(const vector<string>& speclist, string& w_on, string& w_off)
{
std::ostringstream son, soff;
fmt::obufstream son, soff;

for (auto& s: speclist)
{
Expand Down
2 changes: 1 addition & 1 deletion apps/uriparser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ string UriParser::makeUri()
prefix = m_proto + "://";
}

std::ostringstream out;
fmt::obufstream out;

out << prefix << m_host;
if ((m_port == "" || m_port == "0") && m_expect == EXPECT_FILE)
Expand Down
4 changes: 2 additions & 2 deletions srtcore/api.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -231,7 +231,7 @@ string srt::CUDTUnited::CONID(SRTSOCKET sock)
if (sock == 0)
return "";

std::ostringstream os;
fmt::obufstream os;
os << "@" << sock << ":";
return os.str();
}
Expand Down Expand Up @@ -3240,7 +3240,7 @@ bool srt::CUDTUnited::updateListenerMux(CUDTSocket* s, const CUDTSocket* ls)
CMultiplexer& m = i->second;

#if ENABLE_HEAVY_LOGGING
ostringstream that_muxer;
fmt::obufstream that_muxer;
that_muxer << "id=" << m.m_iID << " port=" << m.m_iPort
<< " ip=" << (m.m_iIPversion == AF_INET ? "v4" : "v6");
#endif
Expand Down
15 changes: 6 additions & 9 deletions srtcore/common.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -277,15 +277,12 @@ void srt::CIPAddress::pton(sockaddr_any& w_addr, const uint32_t ip[4], const soc
}
else
{
LOGC(inlog.Error, log << "pton: IPE or net error: can't determine IPv4 carryover format: " << std::hex
<< peeraddr16[0] << ":"
<< peeraddr16[1] << ":"
<< peeraddr16[2] << ":"
<< peeraddr16[3] << ":"
<< peeraddr16[4] << ":"
<< peeraddr16[5] << ":"
<< peeraddr16[6] << ":"
<< peeraddr16[7] << std::dec);
fmt::obufstream peeraddr_form;
peeraddr_form << fmt::sfmt(peeraddr16[0], "04x");
for (int i = 1; i < 8; ++i)
peeraddr_form << ":" << fmt::sfmt(peeraddr16[i], "04x");

LOGC(inlog.Error, log << "pton: IPE or net error: can't determine IPv4 carryover format: " << peeraddr_form);
*target_ipv4_addr = 0;
if (peer.family() != AF_INET)
{
Expand Down
2 changes: 1 addition & 1 deletion srtcore/congctl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -595,7 +595,7 @@ class FileCC : public SrtCongestionControlBase
{
m_dPktSndPeriod = m_dCWndSize / (m_parent->SRTT() + m_iRCInterval);
HLOGC(cclog.Debug, log << "FileCC: CHKTIMER, SLOWSTART:OFF, sndperiod=" << m_dPktSndPeriod << "us AS wndsize/(RTT+RCIV) (wndsize="
<< setprecision(6) << m_dCWndSize << " RTT=" << m_parent->SRTT() << " RCIV=" << m_iRCInterval << ")");
<< fmt::sfmt(m_dCWndSize, ".6") << " RTT=" << m_parent->SRTT() << " RCIV=" << m_iRCInterval << ")");
}
}
else
Expand Down
47 changes: 28 additions & 19 deletions srtcore/core.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -88,10 +88,13 @@ using namespace std;
using namespace srt;
using namespace srt::sync;
using namespace srt_logging;
using fmt::sfmt;

const SRTSOCKET UDT::INVALID_SOCK = srt::CUDT::INVALID_SOCK;
const int UDT::ERROR = srt::CUDT::ERROR;

static const char onoff[2] = {'-', '+'};

//#define SRT_CMD_HSREQ 1 /* SRT Handshake Request (sender) */
#define SRT_CMD_HSREQ_MINSZ 8 /* Minumum Compatible (1.x.x) packet size (bytes) */
#define SRT_CMD_HSREQ_SZ 12 /* Current version packet size */
Expand Down Expand Up @@ -1773,8 +1776,11 @@ bool srt::CUDT::createSrtHandshake(
m_RejectReason = SRT_REJ_IPE;
LOGC(cnlog.Error,
log << CONID() << "createSrtHandshake: IPE: need to send KM, but CryptoControl does not exist."
<< " Socket state: connected=" << boolalpha << m_bConnected << ", connecting=" << m_bConnecting
<< ", broken=" << m_bBroken << ", closing=" << m_bClosing << ".");
<< " Socket state: "
<< onoff[m_bConnected] << "connected, "
<< onoff[m_bConnecting] << "connecting, "
<< onoff[m_bBroken] << "broken, "
<< onoff[m_bClosing] << "closing.");
return false;
}

Expand Down Expand Up @@ -2102,8 +2108,9 @@ int srt::CUDT::processSrtMsg_HSREQ(const uint32_t *srtdata, size_t bytelen, uint
}

LOGC(cnlog.Debug, log << "HSREQ/rcv: cmd=" << SRT_CMD_HSREQ << "(HSREQ) len=" << bytelen
<< hex << " vers=0x" << srtdata[SRT_HS_VERSION] << " opts=0x" << srtdata[SRT_HS_FLAGS]
<< dec << " delay=" << SRT_HS_LATENCY_RCV::unwrap(srtdata[SRT_HS_LATENCY]));
<< " vers=0x" << sfmt(srtdata[SRT_HS_VERSION], "x")
<< " opts=0x" << sfmt(srtdata[SRT_HS_FLAGS], "x")
<< " delay=" << SRT_HS_LATENCY_RCV::unwrap(srtdata[SRT_HS_LATENCY]));

m_uPeerSrtVersion = srtdata[SRT_HS_VERSION];
m_uPeerSrtFlags = srtdata[SRT_HS_FLAGS];
Expand Down Expand Up @@ -2337,8 +2344,8 @@ int srt::CUDT::processSrtMsg_HSRSP(const uint32_t *srtdata, size_t bytelen, uint
m_uPeerSrtFlags = srtdata[SRT_HS_FLAGS];

HLOGC(cnlog.Debug, log << "HSRSP/rcv: Version: " << SrtVersionString(m_uPeerSrtVersion)
<< " Flags: SND:" << setw(8) << setfill('0') << hex << m_uPeerSrtFlags
<< setw(0) << " (" << SrtFlagString(m_uPeerSrtFlags) << ")");
<< " Flags: SND:" << sfmt(m_uPeerSrtFlags, "08x")
<< " (" << SrtFlagString(m_uPeerSrtFlags) << ")");
// Basic version check
if (m_uPeerSrtVersion < m_config.uMinimumPeerSrtVersion)
{
Expand Down Expand Up @@ -4101,8 +4108,12 @@ EConnectStatus srt::CUDT::craftKmResponse(uint32_t* aw_kmdata, size_t& w_kmdatas
m_RejectReason = SRT_REJ_IPE;
LOGC(cnlog.Error,
log << CONID() << "IPE: craftKmResponse needs to send KM, but CryptoControl does not exist."
<< " Socket state: connected=" << boolalpha << m_bConnected << ", connecting=" << m_bConnecting
<< ", broken=" << m_bBroken << ", opened " << m_bOpened << ", closing=" << m_bClosing << ".");
<< " Socket state: "
<< onoff[m_bConnected] << "connected, "
<< onoff[m_bConnecting] << "connecting, "
<< onoff[m_bBroken] << "broken, "
<< onoff[m_bOpened] << "opened, "
<< onoff[m_bClosing] << "closing.");
return CONN_REJECT;
}
// This is a periodic handshake update, so you need to extract the KM data from the
Expand Down Expand Up @@ -5460,14 +5471,14 @@ void * srt::CUDT::tsbpd(void* param)
HLOGC(tslog.Debug,
log << self->CONID() << "tsbpd: DROPSEQ: up to seqno %" << CSeqNo::decseq(info.seqno) << " ("
<< iDropCnt << " packets) playable at " << FormatTime(info.tsbpd_time) << " delayed "
<< (timediff_us / 1000) << "." << std::setw(3) << std::setfill('0') << (timediff_us % 1000) << " ms");
<< (timediff_us / 1000) << "." << sfmt(timediff_us % 1000, "03") << " ms");
#endif
string why;
if (self->frequentLogAllowed(FREQLOGFA_RCV_DROPPED, tnow, (why)))
{
LOGC(brlog.Warn, log << self->CONID() << "RCV-DROPPED " << iDropCnt << " packet(s). Packet seqno %" << info.seqno
<< " delayed for " << (timediff_us / 1000) << "." << std::setw(3) << std::setfill('0')
<< (timediff_us % 1000) << " ms " << why);
<< " delayed for " << (timediff_us / 1000) << "."
<< sfmt(timediff_us % 1000, "03") << " ms " << why);
}
#if SRT_ENABLE_FREQUENT_LOG_TRACE
else
Expand Down Expand Up @@ -7766,8 +7777,7 @@ bool srt::CUDT::updateCC(ETransmissionEvent evt, const EventVariant arg)
#if ENABLE_HEAVY_LOGGING
HLOGC(rslog.Debug,
log << CONID() << "updateCC: updated values from congctl: interval=" << FormatDuration<DUNIT_US>(m_tdSendInterval)
<< " (cfg:" << m_CongCtl->pktSndPeriod_us() << "us) cgwindow="
<< std::setprecision(3) << cgwindow);
<< " (cfg:" << m_CongCtl->pktSndPeriod_us() << "us) cgwindow=" << sfmt(cgwindow, ".3"));
#endif
}

Expand Down Expand Up @@ -8408,7 +8418,7 @@ void srt::CUDT::processCtrlAck(const CPacket &ctrlpkt, const steady_clock::time_
// included, but it also triggers for any other kind of invalid value.
// This check MUST BE DONE before making any operation on this number.
LOGC(inlog.Error, log << CONID() << "ACK: IPE/EPE: received invalid ACK value: " << ackdata_seqno
<< " " << std::hex << ackdata_seqno << " (IGNORED)");
<< " " << sfmt(ackdata_seqno, "x") << " (IGNORED)");
return;
}

Expand Down Expand Up @@ -10002,12 +10012,11 @@ int srt::CUDT::checkLazySpawnTsbPdThread()

HLOGP(qrlog.Debug, "Spawning Socket TSBPD thread");
#if ENABLE_HEAVY_LOGGING
std::ostringstream tns1, tns2;
fmt::obufstream buf;
// Take the last 2 ciphers from the socket ID.
tns1 << setfill('0') << setw(2) << m_SocketID;
std::string s = tns1.str();
tns2 << "SRT:TsbPd:@" << s.substr(s.size()-2, 2);
const string thname = tns2.str();
string s = fmt::sfmts(m_SocketID, "02");
buf << "SRT:TsbPd:@" << s.substr(s.size()-2, 2);
const string thname = buf.str();
#else
const string thname = "SRT:TsbPd";
#endif
Expand Down
6 changes: 4 additions & 2 deletions srtcore/group.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ extern const int32_t SRT_DEF_VERSION;

namespace srt {

static inline char fmt_onoff(bool val) { return val ? '+' : '-'; }

int32_t CUDTGroup::s_tokenGen = 0;

// [[using locked(this->m_GroupLock)]];
Expand Down Expand Up @@ -2190,8 +2192,8 @@ int CUDTGroup::recv(char* buf, int len, SRT_MSGCTRL& w_mc)
if (!m_bOpened || !m_bConnected)
{
LOGC(grlog.Error,
log << boolalpha << "grp/recv: $" << id() << ": ABANDONING: opened=" << m_bOpened
<< " connected=" << m_bConnected);
log << "grp/recv: $" << id() << ": ABANDONING: opened" << fmt_onoff(m_bOpened)
<< " connected" << fmt_onoff(m_bConnected));
throw CUDTException(MJ_CONNECTION, MN_NOCONN, 0);
}

Expand Down
4 changes: 2 additions & 2 deletions srtcore/logging.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ LogDispatcher::Proxy LogDispatcher::operator()()
return Proxy(*this);
}

void LogDispatcher::CreateLogLinePrefix(std::ostringstream& serr)
void LogDispatcher::CreateLogLinePrefix(fmt::obufstream& serr)
{
using namespace std;
using namespace srt;
Expand All @@ -60,7 +60,7 @@ void LogDispatcher::CreateLogLinePrefix(std::ostringstream& serr)

if (strftime(tmp_buf, sizeof(tmp_buf), "%X.", &tm))
{
serr << tmp_buf << setw(6) << setfill('0') << tv.tv_usec;
serr << tmp_buf << fmt::sfmt(tv.tv_usec, "06");
}
}

Expand Down
78 changes: 54 additions & 24 deletions srtcore/logging.h
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@
#include <iostream>
#include <iomanip>
#include <set>
#include <sstream>
#include <cstdarg>
#ifdef _WIN32
#include "win/wintime.h"
Expand All @@ -29,6 +28,8 @@
#include <sys/time.h>
#endif

#include "sfmt.h"

#include "srt.h"
#include "utilities.h"
#include "threadname.h"
Expand Down Expand Up @@ -192,7 +193,7 @@

bool CheckEnabled();

void CreateLogLinePrefix(std::ostringstream&);
void CreateLogLinePrefix(fmt::obufstream&);
void SendLogLine(const char* file, int line, const std::string& area, const std::string& sl);

// log.Debug("This is the ", nth, " time"); <--- C++11 only.
Expand Down Expand Up @@ -285,7 +286,7 @@
{
LogDispatcher& that;

std::ostringstream os;
fmt::obufstream os;

// Cache the 'enabled' state in the beginning. If the logging
// becomes enabled or disabled in the middle of the log, we don't
Expand Down Expand Up @@ -336,12 +337,49 @@
return *this;
}

// Special case for atomics, as passing them to snprintf() call
// requires unpacking the real underlying value.
template <class T>
Proxy& operator<<(const srt::sync::atomic<T>& arg)
{
if ( that_enabled )
{
os << arg.load();
}
return *this;
}


#if HAVE_CXX11

void dispatch() {}

template<typename Arg1, typename... Args>
void dispatch(const Arg1& a1, const Args&... others)
Dismissed Show dismissed Hide dismissed
Dismissed Show dismissed Hide dismissed
{
*this << a1;
dispatch(others...);
}

// Special dispatching for atomics must be provided here.
// By some reason, "*this << a1" expression gets dispatched
// to the general version of operator<<, not the overload for
// atomic. Even though the compiler shows Arg1 type as atomic.
template<typename Arg1, typename... Args>
void dispatch(const srt::sync::atomic<Arg1>& a1, const Args&... others)
{
*this << a1.load();
dispatch(others...);
}

#endif

~Proxy()
{
if ( that_enabled )
{
if ( (flags & SRT_LOGF_DISABLE_EOL) == 0 )
os << std::endl;
os << fmt::seol;
that.SendLogLine(i_file, i_line, area, os.str());
}
// Needed in destructor?
Expand Down Expand Up @@ -435,28 +473,28 @@

//extern std::mutex Debug_mutex;

inline void PrintArgs(std::ostream&) {}
inline void PrintArgs(fmt::obufstream&) {}

template <class Arg1, class... Args>
inline void PrintArgs(std::ostream& serr, Arg1&& arg1, Args&&... args)
inline void PrintArgs(fmt::obufstream& serr, Arg1&& arg1, Args&&... args)
{
serr << std::forward<Arg1>(arg1);
PrintArgs(serr, args...);
}

// Add exceptional handling for sync::atomic
template <class Arg1, class... Args>
inline void PrintArgs(fmt::obufstream& serr, const srt::sync::atomic<Arg1>& arg1, Args&&... args)
{
serr << arg1.load();
PrintArgs(serr, args...);
}

template <class... Args>
inline void LogDispatcher::PrintLogLine(const char* file SRT_ATR_UNUSED, int line SRT_ATR_UNUSED, const std::string& area SRT_ATR_UNUSED, Args&&... args SRT_ATR_UNUSED)
{
#ifdef ENABLE_LOGGING
std::ostringstream serr;
CreateLogLinePrefix(serr);
PrintArgs(serr, args...);

if ( !isset(SRT_LOGF_DISABLE_EOL) )
serr << std::endl;

// Not sure, but it wasn't ever used.
SendLogLine(file, line, area, serr.str());
Proxy(*this).dispatch(args...);
#endif
}

Expand All @@ -466,15 +504,7 @@
inline void LogDispatcher::PrintLogLine(const char* file SRT_ATR_UNUSED, int line SRT_ATR_UNUSED, const std::string& area SRT_ATR_UNUSED, const Arg& arg SRT_ATR_UNUSED)
{
#ifdef ENABLE_LOGGING
std::ostringstream serr;
CreateLogLinePrefix(serr);
serr << arg;

if ( !isset(SRT_LOGF_DISABLE_EOL) )
serr << std::endl;

// Not sure, but it wasn't ever used.
SendLogLine(file, line, area, serr.str());
Proxy(*this) << arg;
#endif
}

Expand Down
4 changes: 2 additions & 2 deletions srtcore/queue.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1078,8 +1078,8 @@ bool srt::CRendezvousQueue::qualifyToHandle(EReadStatus rst,
else
{
HLOGC(cnlog.Debug,
log << "RID: socket @" << i->m_iID << " still active (remaining " << std::fixed
<< (count_microseconds(i->m_tsTTL - tsNow) / 1000000.0) << "s of TTL)...");
log << "RID: socket @" << i->m_iID << " still active (remaining "
<< fmt::sfmt(count_microseconds(i->m_tsTTL - tsNow) / 1000000.0, "f") << "s of TTL)...");
}

const steady_clock::time_point tsLastReq = i->m_pUDT->m_tsLastReqTime;
Expand Down
4 changes: 4 additions & 0 deletions srtcore/queue.h
Original file line number Diff line number Diff line change
Expand Up @@ -592,6 +592,10 @@ struct CMultiplexer
, m_pRcvQueue(NULL)
, m_pChannel(NULL)
, m_pTimer(NULL)
, m_iPort(0)
, m_iIPversion(0)
, m_iRefCount(1)
, m_iID(-1)
{
}

Expand Down
Loading
Loading