table: optimize removal of PIT in-records
Most call sites already have an iterator that can be used directly,
so change deleteInRecord() to accept an iterator instead of doing
another list lookup.
Change-Id: Ie097f4bc4b13e7428e0580ed7cf4dcb506a009f7
diff --git a/daemon/fw/forwarder.cpp b/daemon/fw/forwarder.cpp
index 906c2f2..4aa78e2 100644
--- a/daemon/fw/forwarder.cpp
+++ b/daemon/fw/forwarder.cpp
@@ -377,8 +377,7 @@
pitEntry->deleteOutRecord(ingress.face);
}
- // foreach pending downstream
- for (const auto& pendingDownstream : pendingDownstreams) {
+ for (Face* pendingDownstream : pendingDownstreams) {
if (pendingDownstream->getId() == ingress.face.getId() &&
pendingDownstream->getLinkType() != ndn::nfd::LINK_TYPE_AD_HOC) {
continue;
@@ -520,7 +519,7 @@
nackPkt.setHeader(nack);
// erase in-record
- pitEntry->deleteInRecord(egress);
+ pitEntry->deleteInRecord(inRecord);
// send Nack on face
egress.sendNack(nackPkt);
diff --git a/daemon/fw/strategy.cpp b/daemon/fw/strategy.cpp
index 10610ad..712e200 100644
--- a/daemon/fw/strategy.cpp
+++ b/daemon/fw/strategy.cpp
@@ -248,20 +248,19 @@
{
BOOST_ASSERT(pitEntry->getInterest().matchesData(data));
- shared_ptr<lp::PitToken> pitToken;
auto inRecord = pitEntry->getInRecord(egress);
if (inRecord != pitEntry->in_end()) {
- pitToken = inRecord->getInterest().getTag<lp::PitToken>();
- }
+ auto pitToken = inRecord->getInterest().getTag<lp::PitToken>();
- // delete the PIT entry's in-record based on egress,
- // since the Data is sent to the face from which the Interest was received
- pitEntry->deleteInRecord(egress);
+ // delete the PIT entry's in-record based on egress,
+ // since the Data is sent to the face from which the Interest was received
+ pitEntry->deleteInRecord(inRecord);
- if (pitToken != nullptr) {
- Data data2 = data; // make a copy so each downstream can get a different PIT token
- data2.setTag(pitToken);
- return m_forwarder.onOutgoingData(data2, egress);
+ if (pitToken != nullptr) {
+ Data data2 = data; // make a copy so each downstream can get a different PIT token
+ data2.setTag(pitToken);
+ return m_forwarder.onOutgoingData(data2, egress);
+ }
}
return m_forwarder.onOutgoingData(data, egress);
}
diff --git a/daemon/table/pit-entry.cpp b/daemon/table/pit-entry.cpp
index b302c28..194106d 100644
--- a/daemon/table/pit-entry.cpp
+++ b/daemon/table/pit-entry.cpp
@@ -1,6 +1,6 @@
/* -*- Mode:C++; c-file-style:"gnu"; indent-tabs-mode:nil; -*- */
/*
- * Copyright (c) 2014-2022, Regents of the University of California,
+ * Copyright (c) 2014-2024, Regents of the University of California,
* Arizona Board of Regents,
* Colorado State University,
* University Pierre & Marie Curie, Sorbonne University,
@@ -51,7 +51,7 @@
Entry::getInRecord(const Face& face)
{
return std::find_if(m_inRecords.begin(), m_inRecords.end(),
- [&face] (const InRecord& inRecord) { return &inRecord.getFace() == &face; });
+ [&face] (const InRecord& inRecord) { return &inRecord.getFace() == &face; });
}
InRecordCollection::iterator
@@ -60,7 +60,7 @@
BOOST_ASSERT(this->canMatch(interest));
auto it = std::find_if(m_inRecords.begin(), m_inRecords.end(),
- [&face] (const InRecord& inRecord) { return &inRecord.getFace() == &face; });
+ [&face] (const InRecord& inRecord) { return &inRecord.getFace() == &face; });
if (it == m_inRecords.end()) {
m_inRecords.emplace_front(face);
it = m_inRecords.begin();
@@ -71,13 +71,10 @@
}
void
-Entry::deleteInRecord(const Face& face)
+Entry::deleteInRecord(InRecordCollection::const_iterator pos)
{
- auto it = std::find_if(m_inRecords.begin(), m_inRecords.end(),
- [&face] (const InRecord& inRecord) { return &inRecord.getFace() == &face; });
- if (it != m_inRecords.end()) {
- m_inRecords.erase(it);
- }
+ BOOST_ASSERT(pos != m_inRecords.end());
+ m_inRecords.erase(pos);
}
void
@@ -90,7 +87,7 @@
Entry::getOutRecord(const Face& face)
{
return std::find_if(m_outRecords.begin(), m_outRecords.end(),
- [&face] (const OutRecord& outRecord) { return &outRecord.getFace() == &face; });
+ [&face] (const OutRecord& outRecord) { return &outRecord.getFace() == &face; });
}
OutRecordCollection::iterator
@@ -99,7 +96,7 @@
BOOST_ASSERT(this->canMatch(interest));
auto it = std::find_if(m_outRecords.begin(), m_outRecords.end(),
- [&face] (const OutRecord& outRecord) { return &outRecord.getFace() == &face; });
+ [&face] (const OutRecord& outRecord) { return &outRecord.getFace() == &face; });
if (it == m_outRecords.end()) {
m_outRecords.emplace_front(face);
it = m_outRecords.begin();
@@ -113,7 +110,7 @@
Entry::deleteOutRecord(const Face& face)
{
auto it = std::find_if(m_outRecords.begin(), m_outRecords.end(),
- [&face] (const OutRecord& outRecord) { return &outRecord.getFace() == &face; });
+ [&face] (const OutRecord& outRecord) { return &outRecord.getFace() == &face; });
if (it != m_outRecords.end()) {
m_outRecords.erase(it);
}
diff --git a/daemon/table/pit-entry.hpp b/daemon/table/pit-entry.hpp
index 82ed272..d6f304b 100644
--- a/daemon/table/pit-entry.hpp
+++ b/daemon/table/pit-entry.hpp
@@ -93,10 +93,11 @@
canMatch(const Interest& interest, size_t nEqualNameComps = 0) const;
public: // in-record
- /** \return collection of in-records
+ /**
+ * \brief Returns the collection of in-records.
*/
const InRecordCollection&
- getInRecords() const
+ getInRecords() const noexcept
{
return m_inRecords;
}
@@ -107,62 +108,68 @@
* This implies the entry is new or has been satisfied or Nacked.
*/
bool
- hasInRecords() const
+ hasInRecords() const noexcept
{
return !m_inRecords.empty();
}
InRecordCollection::iterator
- in_begin()
+ in_begin() noexcept
{
return m_inRecords.begin();
}
InRecordCollection::const_iterator
- in_begin() const
+ in_begin() const noexcept
{
return m_inRecords.begin();
}
InRecordCollection::iterator
- in_end()
+ in_end() noexcept
{
return m_inRecords.end();
}
InRecordCollection::const_iterator
- in_end() const
+ in_end() const noexcept
{
return m_inRecords.end();
}
- /** \brief Get the in-record for \p face.
- * \return an iterator to the in-record, or in_end() if it does not exist
+ /**
+ * \brief Get the in-record for \p face.
+ * \return an iterator to the in-record, or in_end() if it does not exist
*/
InRecordCollection::iterator
getInRecord(const Face& face);
- /** \brief Insert or update an in-record.
- * \return an iterator to the new or updated in-record
+ /**
+ * \brief Insert or update an in-record.
+ * \return an iterator to the new or updated in-record
*/
InRecordCollection::iterator
insertOrUpdateInRecord(Face& face, const Interest& interest);
- /** \brief Delete the in-record for \p face if it exists.
+ /**
+ * \brief Removes the in-record at position \p pos.
+ * \param pos iterator to the in-record to remove; must be valid and dereferenceable.
*/
void
- deleteInRecord(const Face& face);
+ deleteInRecord(InRecordCollection::const_iterator pos);
- /** \brief Delete all in-records.
+ /**
+ * \brief Removes all in-records.
*/
void
clearInRecords();
public: // out-record
- /** \return collection of in-records
+ /**
+ * \brief Returns the collection of out-records.
*/
const OutRecordCollection&
- getOutRecords() const
+ getOutRecords() const noexcept
{
return m_outRecords;
}
@@ -174,48 +181,51 @@
* This implies the Interest has not been forwarded.
*/
bool
- hasOutRecords() const
+ hasOutRecords() const noexcept
{
return !m_outRecords.empty();
}
OutRecordCollection::iterator
- out_begin()
+ out_begin() noexcept
{
return m_outRecords.begin();
}
OutRecordCollection::const_iterator
- out_begin() const
+ out_begin() const noexcept
{
return m_outRecords.begin();
}
OutRecordCollection::iterator
- out_end()
+ out_end() noexcept
{
return m_outRecords.end();
}
OutRecordCollection::const_iterator
- out_end() const
+ out_end() const noexcept
{
return m_outRecords.end();
}
- /** \brief Get the out-record for \p face.
- * \return an iterator to the out-record, or out_end() if it does not exist
+ /**
+ * \brief Get the out-record for \p face.
+ * \return an iterator to the out-record, or out_end() if it does not exist
*/
OutRecordCollection::iterator
getOutRecord(const Face& face);
- /** \brief Insert or update an out-record.
- * \return an iterator to the new or updated out-record
+ /**
+ * \brief Insert or update an out-record.
+ * \return an iterator to the new or updated out-record
*/
OutRecordCollection::iterator
insertOrUpdateOutRecord(Face& face, const Interest& interest);
- /** \brief Delete the out-record for \p face if it exists.
+ /**
+ * \brief Delete the out-record for \p face if it exists.
*/
void
deleteOutRecord(const Face& face);
@@ -232,7 +242,7 @@
bool isSatisfied = false;
/** \brief Data freshness period.
- * \note This field is meaningful only if isSatisfied is true
+ * \note This field is meaningful only if #isSatisfied is true
*/
time::milliseconds dataFreshnessPeriod = 0_ms;
diff --git a/daemon/table/pit.cpp b/daemon/table/pit.cpp
index 4ebdefb..b731df0 100644
--- a/daemon/table/pit.cpp
+++ b/daemon/table/pit.cpp
@@ -1,6 +1,6 @@
/* -*- Mode:C++; c-file-style:"gnu"; indent-tabs-mode:nil; -*- */
/*
- * Copyright (c) 2014-2022, Regents of the University of California,
+ * Copyright (c) 2014-2024, Regents of the University of California,
* Arizona Board of Regents,
* Colorado State University,
* University Pierre & Marie Curie, Sorbonne University,
@@ -115,7 +115,10 @@
{
BOOST_ASSERT(entry != nullptr);
- entry->deleteInRecord(face);
+ auto in = entry->getInRecord(face);
+ if (in != entry->in_end()) {
+ entry->deleteInRecord(in);
+ }
entry->deleteOutRecord(face);
/// \todo decide whether to delete PIT entry if there's no more in/out-record left