Skip to content

Commit

Permalink
[core] Minor ACK variables clean up.
Browse files Browse the repository at this point in the history
* m_ullACKInt_tk used at initialization
* Removed unused m_iACKPeriod from FileCC
* ACK event variant type changed to signed integer
* SrtCongestionControlBase: renamed ACKInterval() -> ACKMaxPackets()
* SrtCongestionControlBase: renamed ACKPeriod() -> ACKTimeout_us()
  • Loading branch information
maxsharabayko authored and rndi committed Sep 13, 2019
1 parent d90aef0 commit e0fbec5
Show file tree
Hide file tree
Showing 8 changed files with 28 additions and 43 deletions.
6 changes: 3 additions & 3 deletions srtcore/common.h
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,7 @@ struct EventVariant
union U
{
CPacket* packet;
uint32_t ack;
int32_t ack;
struct
{
int32_t* ptr;
Expand Down Expand Up @@ -238,7 +238,7 @@ struct EventVariant
}

void operator=(CPacket* arg) { Assign<PACKET>(arg); };
void operator=(uint32_t arg) { Assign<ACK>(arg); };
void operator=(int32_t arg) { Assign<ACK>(arg); };
void operator=(ECheckTimerStage arg) { Assign<STAGE>(arg); };
void operator=(EInitEvent arg) { Assign<INIT>(arg); };

Expand Down Expand Up @@ -323,7 +323,7 @@ template<> struct EventVariant::VariantFor<EventVariant::PACKET>

template<> struct EventVariant::VariantFor<EventVariant::ACK>
{
typedef uint32_t type;
typedef int32_t type;
static type U::*field() { return &U::ack; }
};

Expand Down
8 changes: 1 addition & 7 deletions srtcore/congctl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -248,9 +248,6 @@ class FileCC : public SrtCongestionControlBase
{
typedef FileCC Me; // Required by SSLOT macro

// Fields from CCC not used by FileCC
int m_iACKPeriod;

// Fields from CUDTCC
int m_iRCInterval; // UDT Rate control interval
uint64_t m_LastRCTime; // last rate increase time
Expand All @@ -270,7 +267,6 @@ class FileCC : public SrtCongestionControlBase

FileCC(CUDT* parent)
: SrtCongestionControlBase(parent)
, m_iACKPeriod(CUDT::COMM_SYN_INTERVAL_US)
, m_iRCInterval(CUDT::COMM_SYN_INTERVAL_US)
, m_LastRCTime(CTimer::getTime())
, m_bSlowStart(true)
Expand Down Expand Up @@ -497,13 +493,11 @@ class FileCC : public SrtCongestionControlBase
m_bLoss = true;

const int pktsInFlight = m_parent->RTT() / m_dPktSndPeriod;
const int ackSeqno = m_iLastAck;// m_parent->sndLastDataAck();
const int sentSeqno = m_parent->sndSeqNo();
const int numPktsLost = m_parent->sndLossLength();
const int lost_pcent_x10 = (numPktsLost * 1000) / pktsInFlight;

HLOGC(mglog.Debug, log << "FileSmootherV2: LOSS: "
<< "sent=" << CSeqNo::seqlen(ackSeqno, sentSeqno) << ", inFlight=" << pktsInFlight
<< "sent=" << CSeqNo::seqlen(m_iLastAck, m_parent->sndSeqNo()) << ", inFlight=" << pktsInFlight
<< ", lost=" << numPktsLost << " ("
<< lost_pcent_x10 / 10 << "." << lost_pcent_x10 % 10 << "\%)");
if (lost_pcent_x10 < 20) // 2.0%
Expand Down
20 changes: 9 additions & 11 deletions srtcore/congctl.h
Original file line number Diff line number Diff line change
Expand Up @@ -156,19 +156,17 @@ class SrtCongestionControlBase
// If not, it will be internally calculated.
virtual int RTO() { return 0; }

// How many packets to send one ACK, in packets.
// If user-defined, will return nonzero value. It can enforce extra ACKs
// beside those calculated by ACK, sent only when the number of packets
// received since the last EXP that fired "fat" ACK does not exceed this
// value.
virtual int ACKInterval() { return 0; }

// Periodical timer to send an ACK, in microseconds.
// If user-defined, this value in microseconds will be used to calculate
// Maximum number of packets to trigger ACK sending.
// Specifies the number of packets to receive before sending the ACK.
// Used by CUDT together with ACKTimeout_us() to trigger ACK packet sending.
virtual int ACKMaxPackets() const { return 0; }

// Periodical interval to send an ACK, in microseconds.
// If user-defined, this value will be used to calculate
// the next ACK time every time ACK is considered to be sent (see CUDT::checkTimers).
// Otherwise this will be calculated internally in CUDT, normally taken
// from CPacket::SYN_INTERVAL.
virtual int ACKPeriod() { return 0; }
// from CUDT::COMM_SYN_INTERVAL_US.
virtual int ACKTimeout_us() const { return 0; }

// Called when the settings concerning m_llMaxBW were changed.
// Arg 1: value of CUDT::m_llMaxBW
Expand Down
25 changes: 12 additions & 13 deletions srtcore/core.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1371,8 +1371,6 @@ void CUDT::open()
m_iRTTVar = m_iRTT >> 1;
m_ullCPUFrequency = CTimer::getCPUFrequency();

// set up the timers
m_ullSYNInt_tk = COMM_SYN_INTERVAL_US * m_ullCPUFrequency;

// set minimum NAK and EXP timeout to 300ms
/*
Expand All @@ -1387,16 +1385,17 @@ void CUDT::open()
else
#endif
*/
// Set up timers
m_ullMinNakInt_tk = 300000 * m_ullCPUFrequency;
m_ullMinExpInt_tk = 300000 * m_ullCPUFrequency;

m_ullACKInt_tk = m_ullSYNInt_tk;
m_ullACKInt_tk = COMM_SYN_INTERVAL_US * m_ullCPUFrequency;
m_ullNAKInt_tk = m_ullMinNakInt_tk;

uint64_t currtime_tk;
CTimer::rdtsc(currtime_tk);
m_ullLastRspTime_tk = currtime_tk;
m_ullNextACKTime_tk = currtime_tk + m_ullSYNInt_tk;
m_ullNextACKTime_tk = currtime_tk + m_ullACKInt_tk;
m_ullNextNAKTime_tk = currtime_tk + m_ullNAKInt_tk;
m_ullLastRspAckTime_tk = currtime_tk;
m_ullLastSndTime_tk = currtime_tk;
Expand Down Expand Up @@ -5090,7 +5089,7 @@ SRT_REJECT_REASON CUDT::setupCC()
uint64_t currtime_tk;
CTimer::rdtsc(currtime_tk);
m_ullLastRspTime_tk = currtime_tk;
m_ullNextACKTime_tk = currtime_tk + m_ullSYNInt_tk;
m_ullNextACKTime_tk = currtime_tk + m_ullACKInt_tk;
m_ullNextNAKTime_tk = currtime_tk + m_ullNAKInt_tk;
m_ullLastRspAckTime_tk = currtime_tk;
m_ullLastSndTime_tk = currtime_tk;
Expand Down Expand Up @@ -6956,7 +6955,8 @@ void CUDT::sendCtrl(UDTMessageType pkttype, void* lparam, void* rparam, int size
if (data[ACKD_BUFFERLEFT] < 2)
data[ACKD_BUFFERLEFT] = 2;

if (currtime_tk - m_ullLastAckTime_tk > m_ullSYNInt_tk)
// NOTE: m_CongCtl->ACKTimeout_us() should be taken into account.
if (currtime_tk - m_ullLastAckTime_tk > m_ullACKInt_tk)
{
int rcvRate;
int ctrlsz = ACKD_TOTAL_SIZE_UDTBASE * ACKD_FIELD_SIZE; // Minimum required size
Expand All @@ -6976,7 +6976,6 @@ void CUDT::sendCtrl(UDTMessageType pkttype, void* lparam, void* rparam, int size
// Normal, currently expected version.
data[ACKD_RCVRATE] = rcvRate; //bytes/sec
ctrlsz = ACKD_FIELD_SIZE * ACKD_TOTAL_SIZE_VER101;

}
// ELSE: leave the buffer with ...UDTBASE size.

Expand Down Expand Up @@ -9185,17 +9184,17 @@ void CUDT::addLossRecord(std::vector<int32_t>& lr, int32_t lo, int32_t hi)
void CUDT::checkACKTimer(uint64_t currtime_tk)
{
if (currtime_tk > m_ullNextACKTime_tk // ACK time has come
// OR the number of sent packets since last ACK has reached
// the congctl-defined value of ACK Interval
// (note that none of the builtin congctls defines ACK Interval)
|| (m_CongCtl->ACKInterval() > 0 && m_iPktCount >= m_CongCtl->ACKInterval()))
// OR the number of sent packets since last ACK has reached
// the congctl-defined value of ACK Interval
// (note that none of the builtin congctls defines ACK Interval)
|| (m_CongCtl->ACKMaxPackets() > 0 && m_iPktCount >= m_CongCtl->ACKMaxPackets()))
{
// ACK timer expired or ACK interval is reached
sendCtrl(UMSG_ACK);
CTimer::rdtsc(currtime_tk);

const int ack_interval_tk = m_CongCtl->ACKPeriod() > 0
? m_CongCtl->ACKPeriod() * m_ullCPUFrequency
const int ack_interval_tk = m_CongCtl->ACKTimeout_us() > 0
? m_CongCtl->ACKTimeout_us() * m_ullCPUFrequency
: m_ullACKInt_tk;
m_ullNextACKTime_tk = currtime_tk + ack_interval_tk;

Expand Down
1 change: 0 additions & 1 deletion srtcore/core.h
Original file line number Diff line number Diff line change
Expand Up @@ -822,7 +822,6 @@ class CUDT
uint64_t m_ullNextACKTime_tk; // Next ACK time, in CPU clock cycles, same below
uint64_t m_ullNextNAKTime_tk; // Next NAK time

volatile uint64_t m_ullSYNInt_tk; // SYN interval
volatile uint64_t m_ullACKInt_tk; // ACK interval
volatile uint64_t m_ullNAKInt_tk; // NAK interval
volatile uint64_t m_ullLastRspTime_tk; // time stamp of last response from the peer
Expand Down
2 changes: 1 addition & 1 deletion srtcore/fec.h
Original file line number Diff line number Diff line change
Expand Up @@ -242,7 +242,7 @@ class FECFilterBuiltin: public SrtPacketFilterBase
// So extra 4 bytes are needed, 2 for flags, 2 for length clip.
static const size_t EXTRA_SIZE = 4;

virtual SRT_ARQLevel arqLevel() { return m_fallback_level; }
virtual SRT_ARQLevel arqLevel() ATR_OVERRIDE { return m_fallback_level; }
};

#endif
2 changes: 1 addition & 1 deletion srtcore/packet.h
Original file line number Diff line number Diff line change
Expand Up @@ -385,7 +385,7 @@ friend class CRcvQueue;
int32_t& m_iSeqNo; // alias: sequence number
int32_t& m_iMsgNo; // alias: message number
int32_t& m_iTimeStamp; // alias: timestamp
int32_t& m_iID; // alias: socket ID
int32_t& m_iID; // alias: socket ID
char*& m_pcData; // alias: data/control information

//static const int m_iPktHdrSize; // packet header size
Expand Down
7 changes: 1 addition & 6 deletions test/test_epoll.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,6 @@ TEST(CEPoll, WaitEmptyCall)
SRTSOCKET client_sock = srt_socket(AF_INET, SOCK_DGRAM, 0);
ASSERT_NE(client_sock, SRT_ERROR);

const int yes = 1;
const int no = 0;
ASSERT_NE(srt_setsockopt(client_sock, 0, SRTO_RCVSYN, &no, sizeof no), SRT_ERROR); // for async connect
ASSERT_NE(srt_setsockopt(client_sock, 0, SRTO_SNDSYN, &no, sizeof no), SRT_ERROR); // for async connect
Expand All @@ -96,7 +95,6 @@ TEST(CEPoll, UWaitEmptyCall)
SRTSOCKET client_sock = srt_socket(AF_INET, SOCK_DGRAM, 0);
ASSERT_NE(client_sock, SRT_ERROR);

const int yes = 1;
const int no = 0;
ASSERT_NE(srt_setsockopt(client_sock, 0, SRTO_RCVSYN, &no, sizeof no), SRT_ERROR); // for async connect
ASSERT_NE(srt_setsockopt(client_sock, 0, SRTO_SNDSYN, &no, sizeof no), SRT_ERROR); // for async connect
Expand Down Expand Up @@ -186,12 +184,9 @@ TEST(CEPoll, WrongEpoll_idOnAddUSock)
SRTSOCKET client_sock = srt_socket(AF_INET, SOCK_DGRAM, 0);
ASSERT_NE(client_sock, SRT_ERROR);

const int yes = 1;
const int no = 0;
const int no = 0;
ASSERT_NE(srt_setsockopt(client_sock, 0, SRTO_RCVSYN, &no, sizeof no), SRT_ERROR); // for async connect
ASSERT_NE(srt_setsockopt(client_sock, 0, SRTO_SNDSYN, &no, sizeof no), SRT_ERROR); // for async connect
ASSERT_NE(srt_setsockflag(client_sock, SRTO_SENDER, &yes, sizeof yes), SRT_ERROR);
ASSERT_NE(srt_setsockopt(client_sock, 0, SRTO_TSBPDMODE, &yes, sizeof yes), SRT_ERROR);

const int epoll_id = srt_epoll_create();
ASSERT_GE(epoll_id, 0);
Expand Down

0 comments on commit e0fbec5

Please sign in to comment.