Skip to content

const-ify quic/http3 code #7079

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.

Sign up for GitHub

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

Merged
randall merged 1 commit into from
Aug 4, 2020
Merged

const-ify quic/http3 code #7079

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
6 changes: 3 additions & 3 deletions iocore/net/quic/QUICAckFrameCreator.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -209,7 +209,7 @@ QUICAckFrameManager::QUICAckFrameCreator::push_back(QUICPacketNumber packet_numb
}

size_t
QUICAckFrameManager::QUICAckFrameCreator::size()
QUICAckFrameManager::QUICAckFrameCreator::size() const
{
return this->_packet_numbers.size();
}
Expand All @@ -227,13 +227,13 @@ QUICAckFrameManager::QUICAckFrameCreator::clear()
}

QUICPacketNumber
QUICAckFrameManager::QUICAckFrameCreator::largest_ack_number()
QUICAckFrameManager::QUICAckFrameCreator::largest_ack_number() const
{
return this->_largest_ack_number;
}

ink_hrtime
QUICAckFrameManager::QUICAckFrameCreator::largest_ack_received_time()
QUICAckFrameManager::QUICAckFrameCreator::largest_ack_received_time() const
{
return this->_largest_ack_received_time;
}
Expand Down
6 changes: 3 additions & 3 deletions iocore/net/quic/QUICAckFrameCreator.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ class QUICAckFrameManager : public QUICFrameGenerator
~QUICAckFrameCreator();

void push_back(QUICPacketNumber packet_number, size_t size, bool ack_only);
size_t size();
size_t size() const;
void clear();
void sort();
void forget(QUICPacketNumber largest_acknowledged);
Expand All @@ -58,8 +58,8 @@ class QUICAckFrameManager : public QUICFrameGenerator
// refresh state when frame lost
void refresh_state();

QUICPacketNumber largest_ack_number();
ink_hrtime largest_ack_received_time();
QUICPacketNumber largest_ack_number() const;
ink_hrtime largest_ack_received_time() const;

private:
uint64_t _calculate_delay();
Expand Down
2 changes: 1 addition & 1 deletion iocore/net/quic/QUICApplication.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ QUICStreamIO::consume(int64_t len)
}

bool
QUICStreamIO::is_read_done()
QUICStreamIO::is_read_done() const
{
return this->_read_vio->ntodo() == 0;
}
Expand Down
2 changes: 1 addition & 1 deletion iocore/net/quic/QUICApplication.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ class QUICStreamIO
int64_t read(uint8_t *buf, int64_t len);
int64_t peek(uint8_t *buf, int64_t len);
void consume(int64_t len);
bool is_read_done();
bool is_read_done() const;
virtual void read_reenable();

int64_t write(const uint8_t *buf, int64_t len);
Expand Down
2 changes: 1 addition & 1 deletion iocore/net/quic/QUICFlowController.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ QUICRateAnalyzer::update(QUICOffset offset)
}

uint64_t
QUICRateAnalyzer::expect_recv_bytes(ink_hrtime time)
QUICRateAnalyzer::expect_recv_bytes(ink_hrtime time) const
{
return static_cast<uint64_t>(time * this->_rate);
}
Expand Down
2 changes: 1 addition & 1 deletion iocore/net/quic/QUICFlowController.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ class QUICRateAnalyzer
{
public:
void update(QUICOffset offset);
uint64_t expect_recv_bytes(ink_hrtime time);
uint64_t expect_recv_bytes(ink_hrtime time) const;

private:
double _rate = 0.0;
Expand Down
5 changes: 3 additions & 2 deletions iocore/net/quic/QUICFrame.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -895,7 +895,7 @@ QUICAckFrame::AckBlockSection::first_ack_block() const
}

void
QUICAckFrame::AckBlockSection::add_ack_block(AckBlock block)
QUICAckFrame::AckBlockSection::add_ack_block(const AckBlock &block)
{
this->_ack_blocks.push_back(block);
}
Expand Down Expand Up @@ -2960,7 +2960,8 @@ QUICFrameFactory::create_connection_close_frame(uint8_t *buf, uint16_t error_cod
}

QUICConnectionCloseFrame *
QUICFrameFactory::create_connection_close_frame(uint8_t *buf, QUICConnectionError &error, QUICFrameId id, QUICFrameGenerator *owner)
QUICFrameFactory::create_connection_close_frame(uint8_t *buf, const QUICConnectionError &error, QUICFrameId id,
QUICFrameGenerator *owner)
{
ink_assert(error.cls == QUICErrorClass::TRANSPORT);
if (error.msg) {
Expand Down
4 changes: 2 additions & 2 deletions iocore/net/quic/QUICFrame.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -234,7 +234,7 @@ class QUICAckFrame : public QUICFrame
size_t size() const;
Ptr<IOBufferBlock> to_io_buffer_block(size_t limit) const;
uint64_t first_ack_block() const;
void add_ack_block(const AckBlock block);
void add_ack_block(const AckBlock &block);
Copy link
Contributor

@ywkaras ywkaras Aug 4, 2020

Choose a reason for hiding this comment

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

Nitpick: AckBlock is 16 bytes in size, looks to be small enough to be passed in registers ( https://godbolt.org/z/MdrabW ). So this is a small deoptimization.

Copy link
Member

@maskit maskit Aug 4, 2020

Choose a reason for hiding this comment

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

Was this change made by hand or some tools?
While I want to make Ack processing fast as possible as we can, I also want to take the advantage of auto-correct (if it was done by tools). I guess it beats the unfortunate case that we accidentally copy big objects.

Copy link
Contributor

@ywkaras ywkaras Aug 4, 2020

Choose a reason for hiding this comment

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

Yes you can make an argument that, except in rare cases, it's overoptimizing to pass class types by value. It's future safe if new member variables get added to the class.

const_iterator begin() const;
const_iterator end() const;
bool has_protected() const;
Expand Down Expand Up @@ -800,7 +800,7 @@ class QUICFrameFactory
const char *reason_phrase = nullptr, QUICFrameId id = 0,
QUICFrameGenerator *owner = nullptr);

static QUICConnectionCloseFrame *create_connection_close_frame(uint8_t *buf, QUICConnectionError &error, QUICFrameId id = 0,
static QUICConnectionCloseFrame *create_connection_close_frame(uint8_t *buf, const QUICConnectionError &error, QUICFrameId id = 0,
QUICFrameGenerator *owner = nullptr);

/*
Expand Down
4 changes: 2 additions & 2 deletions iocore/net/quic/QUICFrameRetransmitter.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ struct QUICFrameInformation {
};

struct QUICFrameInformationDeleter {
void operator()(QUICFrameInformation *p);
void operator()(QUICFrameInformation *p) const;
};

using QUICFrameInformationUPtr = std::unique_ptr<QUICFrameInformation, QUICFrameInformationDeleter>;
Expand Down Expand Up @@ -90,7 +90,7 @@ class QUICFrameRetransmitter
extern ClassAllocator<QUICFrameInformation> quicFrameInformationAllocator;

inline void
QUICFrameInformationDeleter::operator()(QUICFrameInformation *p)
QUICFrameInformationDeleter::operator()(QUICFrameInformation *p) const
{
quicFrameInformationAllocator.free(p);
}
6 changes: 3 additions & 3 deletions iocore/net/quic/QUICHandshake.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -212,20 +212,20 @@ QUICHandshake::has_remote_tp() const
}

QUICVersion
QUICHandshake::negotiated_version()
QUICHandshake::negotiated_version() const
{
return this->_version_negotiator->negotiated_version();
}

// Similar to SSLNetVConnection::getSSLCipherSuite()
const char *
QUICHandshake::negotiated_cipher_suite()
QUICHandshake::negotiated_cipher_suite() const
{
return this->_hs_protocol->negotiated_cipher_suite();
}

void
QUICHandshake::negotiated_application_name(const uint8_t **name, unsigned int *len)
QUICHandshake::negotiated_application_name(const uint8_t **name, unsigned int *len) const
{
this->_hs_protocol->negotiated_application_name(name, len);
}
Expand Down
6 changes: 3 additions & 3 deletions iocore/net/quic/QUICHandshake.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,9 +73,9 @@ class QUICHandshake : public QUICFrameHandler, public QUICFrameGenerator
bool check_remote_transport_parameters();

// Getters
QUICVersion negotiated_version();
const char *negotiated_cipher_suite();
void negotiated_application_name(const uint8_t **name, unsigned int *len);
QUICVersion negotiated_version() const;
const char *negotiated_cipher_suite() const;
void negotiated_application_name(const uint8_t **name, unsigned int *len) const;
std::shared_ptr<const QUICTransportParameters> local_transport_parameters();
std::shared_ptr<const QUICTransportParameters> remote_transport_parameters();

Expand Down
10 changes: 5 additions & 5 deletions iocore/net/quic/QUICLossDetector.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ QUICLossDetector::handle_frame(QUICEncryptionLevel level, const QUICFrame &frame
}

QUICPacketNumber
QUICLossDetector::largest_acked_packet_number(QUICPacketNumberSpace pn_space)
QUICLossDetector::largest_acked_packet_number(QUICPacketNumberSpace pn_space) const
{
int index = static_cast<int>(pn_space);
return this->_largest_acked_packet[index];
Expand Down Expand Up @@ -252,7 +252,7 @@ QUICLossDetector::_on_ack_received(const QUICAckFrame &ack_frame, QUICPacketNumb

// If the largest acknowledged is newly acked and
// ack-eliciting, update the RTT.
auto &largest_acked = newly_acked_packets[0];
const auto &largest_acked = newly_acked_packets[0];
if (largest_acked->packet_number == ack_frame.largest_acknowledged() && this->_include_ack_eliciting(newly_acked_packets)) {
ink_hrtime latest_rtt = Thread::get_hrtime() - largest_acked->time_sent;
// _latest_rtt is nanosecond but ack_frame.ack_delay is microsecond and scaled
Expand Down Expand Up @@ -372,7 +372,7 @@ QUICLossDetector::_get_pto_time_and_space(QUICPacketNumberSpace &space)
}

bool
QUICLossDetector::_peer_completed_address_validation()
QUICLossDetector::_peer_completed_address_validation() const
{
return this->_context.connection_info()->is_address_validation_completed();
}
Expand Down Expand Up @@ -592,12 +592,12 @@ QUICLossDetector::_send_one_ack_eliciting_padded_initial_packet()
// ===== Functions below are helper functions =====

void
QUICLossDetector::_retransmit_lost_packet(QUICSentPacketInfo &packet_info)
QUICLossDetector::_retransmit_lost_packet(const QUICSentPacketInfo &packet_info)
{
SCOPED_MUTEX_LOCK(lock, this->_loss_detection_mutex, this_ethread());

QUICLDDebug("Retransmit %s packet #%" PRIu64, QUICDebugNames::packet_type(packet_info.type), packet_info.packet_number);
for (QUICSentPacketInfo::FrameInfo &frame_info : packet_info.frames) {
for (const QUICSentPacketInfo::FrameInfo &frame_info : packet_info.frames) {
auto reactor = frame_info.generated_by();
if (reactor == nullptr) {
continue;
Expand Down
6 changes: 3 additions & 3 deletions iocore/net/quic/QUICLossDetector.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ class QUICLossDetector : public Continuation, public QUICFrameHandler
// OnPacketNumberSpaceDiscarded is on Congestion Control section but having it here makes more sense because most processes are
// for LD.
void on_packet_number_space_discarded(QUICPacketNumberSpace pn_space);
QUICPacketNumber largest_acked_packet_number(QUICPacketNumberSpace pn_space);
QUICPacketNumber largest_acked_packet_number(QUICPacketNumberSpace pn_space) const;
void update_ack_delay_exponent(uint8_t ack_delay_exponent);
void reset();

Expand Down Expand Up @@ -109,11 +109,11 @@ class QUICLossDetector : public Continuation, public QUICFrameHandler
std::map<QUICPacketNumber, QUICSentPacketInfoUPtr> _detect_and_remove_lost_packets(QUICPacketNumberSpace pn_space);
void _set_loss_detection_timer();
void _on_loss_detection_timeout();
void _retransmit_lost_packet(QUICSentPacketInfo &packet_info);
void _retransmit_lost_packet(const QUICSentPacketInfo &packet_info);

ink_hrtime _get_loss_time_and_space(QUICPacketNumberSpace &space);
ink_hrtime _get_pto_time_and_space(QUICPacketNumberSpace &space);
bool _peer_completed_address_validation();
bool _peer_completed_address_validation() const;

std::vector<QUICSentPacketInfoUPtr> _detect_and_remove_acked_packets(const QUICAckFrame &ack_frame,
QUICPacketNumberSpace pn_space);
Expand Down
2 changes: 1 addition & 1 deletion iocore/net/quic/QUICNewRenoCongestionController.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ QUICNewRenoCongestionController::on_packet_sent(size_t bytes_sent)
}

bool
QUICNewRenoCongestionController::_in_congestion_recovery(ink_hrtime sent_time)
QUICNewRenoCongestionController::_in_congestion_recovery(ink_hrtime sent_time) const
{
return sent_time <= this->_congestion_recovery_start_time;
}
Expand Down
2 changes: 1 addition & 1 deletion iocore/net/quic/QUICNewRenoCongestionController.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ class QUICNewRenoCongestionController : public QUICCongestionController
bool _check_credit() const;

// Appendix B. Congestion Control Pseudocode
bool _in_congestion_recovery(ink_hrtime sent_time);
bool _in_congestion_recovery(ink_hrtime sent_time) const;
void _congestion_event(ink_hrtime sent_time);
bool _in_persistent_congestion(const std::map<QUICPacketNumber, QUICSentPacketInfoUPtr> &lost_packets,
const QUICSentPacketInfoUPtr &largest_lost_packet);
Expand Down
27 changes: 15 additions & 12 deletions iocore/net/quic/QUICPacket.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -333,8 +333,8 @@ QUICPacketR::read_essential_info(Ptr<IOBufferBlock> block, QUICPacketType &type,
//
// QUICLongHeaderPacket
//
QUICLongHeaderPacket::QUICLongHeaderPacket(QUICVersion version, QUICConnectionId dcid, QUICConnectionId scid, bool ack_eliciting,
bool probing, bool crypto)
QUICLongHeaderPacket::QUICLongHeaderPacket(QUICVersion version, const QUICConnectionId &dcid, const QUICConnectionId &scid,
bool ack_eliciting, bool probing, bool crypto)
: QUICPacket(ack_eliciting, probing), _version(version), _dcid(dcid), _scid(scid), _is_crypto_packet(crypto)
{
}
Expand Down Expand Up @@ -567,7 +567,7 @@ QUICLongHeaderPacketR::packet_number_offset(size_t &pn_offset, const uint8_t *pa
//
// QUICShortHeaderPacket
//
QUICShortHeaderPacket::QUICShortHeaderPacket(QUICConnectionId dcid, QUICPacketNumber packet_number,
QUICShortHeaderPacket::QUICShortHeaderPacket(const QUICConnectionId &dcid, QUICPacketNumber packet_number,
QUICPacketNumber base_packet_number, QUICKeyPhase key_phase, bool ack_eliciting,
bool probing)
: QUICPacket(ack_eliciting, probing), _dcid(dcid), _packet_number(packet_number), _key_phase(key_phase)
Expand Down Expand Up @@ -883,7 +883,8 @@ QUICStatelessResetPacketR::destination_cid() const
//
// QUICVersionNegotiationPacket
//
QUICVersionNegotiationPacket::QUICVersionNegotiationPacket(QUICConnectionId dcid, QUICConnectionId scid,

QUICVersionNegotiationPacket::QUICVersionNegotiationPacket(const QUICConnectionId &dcid, const QUICConnectionId &scid,
const QUICVersion versions[], int nversions,
QUICVersion version_in_initial)
: QUICLongHeaderPacket(0, dcid, scid, false, false, false),
Expand Down Expand Up @@ -1080,9 +1081,9 @@ QUICVersionNegotiationPacketR::nversions() const
//
// QUICInitialPacket
//
QUICInitialPacket::QUICInitialPacket(QUICVersion version, QUICConnectionId dcid, QUICConnectionId scid, size_t token_len,
ats_unique_buf token, size_t length, QUICPacketNumber packet_number, bool ack_eliciting,
bool probing, bool crypto)
QUICInitialPacket::QUICInitialPacket(QUICVersion version, const QUICConnectionId &dcid, const QUICConnectionId &scid,
size_t token_len, ats_unique_buf token, size_t length, QUICPacketNumber packet_number,
bool ack_eliciting, bool probing, bool crypto)
: QUICLongHeaderPacket(version, dcid, scid, ack_eliciting, probing, crypto),
_token_len(token_len),
_token(std::move(token)),
Expand Down Expand Up @@ -1318,7 +1319,7 @@ QUICInitialPacketR::token_length(size_t &token_length, uint8_t &field_len, size_
//
// QUICZeroRttPacket
//
QUICZeroRttPacket::QUICZeroRttPacket(QUICVersion version, QUICConnectionId dcid, QUICConnectionId scid, size_t length,
QUICZeroRttPacket::QUICZeroRttPacket(QUICVersion version, const QUICConnectionId &dcid, const QUICConnectionId &scid, size_t length,
QUICPacketNumber packet_number, bool ack_eliciting, bool probing)
: QUICLongHeaderPacket(version, dcid, scid, ack_eliciting, probing, false), _packet_number(packet_number)
{
Expand Down Expand Up @@ -1497,8 +1498,9 @@ QUICZeroRttPacketR::attach_payload(Ptr<IOBufferBlock> payload, bool unprotected)
//
// QUICHandshakePacket
//
QUICHandshakePacket::QUICHandshakePacket(QUICVersion version, QUICConnectionId dcid, QUICConnectionId scid, size_t length,
QUICPacketNumber packet_number, bool ack_eliciting, bool probing, bool crypto)
QUICHandshakePacket::QUICHandshakePacket(QUICVersion version, const QUICConnectionId &dcid, const QUICConnectionId &scid,
size_t length, QUICPacketNumber packet_number, bool ack_eliciting, bool probing,
bool crypto)
: QUICLongHeaderPacket(version, dcid, scid, ack_eliciting, probing, crypto), _packet_number(packet_number)
{
}
Expand Down Expand Up @@ -1676,7 +1678,8 @@ QUICHandshakePacketR::attach_payload(Ptr<IOBufferBlock> payload, bool unprotecte
//
// QUICRetryPacket
//
QUICRetryPacket::QUICRetryPacket(QUICVersion version, QUICConnectionId dcid, QUICConnectionId scid, QUICRetryToken &token)
QUICRetryPacket::QUICRetryPacket(QUICVersion version, const QUICConnectionId &dcid, const QUICConnectionId &scid,
QUICRetryToken &token)
: QUICLongHeaderPacket(version, dcid, scid, false, false, false), _token(token)
{
}
Expand Down Expand Up @@ -1841,7 +1844,7 @@ QUICRetryPacketR::token() const
}

bool
QUICRetryPacketR::has_valid_tag(QUICConnectionId &odcid) const
QUICRetryPacketR::has_valid_tag(const QUICConnectionId &odcid) const
{
uint8_t tag_computed[QUICRetryIntegrityTag::LEN];
QUICRetryIntegrityTag::compute(tag_computed, this->version(), odcid, this->_header_block, this->_payload_block_without_tag);
Expand Down
Loading