commit e7bb6c8c08f4e154bf51546509b7f431418d6550
author Junxiao Shi <git@mail1.yoursunny.com> Mon Aug 08 23:16:35 2016 +0000
committer Junxiao Shi <git@mail1.yoursunny.com> Mon Aug 08 23:16:35 2016 +0000
tree c25c6d30fc2fdb7957e1956085f0670f28d582a6
parent ae0b418784aa1b021797190d0e56abb95a386b3c

face: perform wire format encoding in frontend class

Face::put encodes Data and Nack into wire format before passing to Impl class,
so that Face::put(const Data&) doesn't require the Data to be managed by a
shared_ptr.
For the common case of sending Data without other NDNLPv2 headers, this commit
adds an optimization that the Data is not copied into an LpPacket, but directly
encodes into wire format.

refs #3678

Change-Id: I7ac7bb574a5cb3b07f6c62060809c94ff1cf3dee

src/detail/face-impl.hpp

diff --git a/src/detail/face-impl.hpp b/src/detail/face-impl.hpp
index fa5f989..92f102f 100644
--- a/src/detail/face-impl.hpp
+++ b/src/detail/face-impl.hpp
@@ -173,35 +173,10 @@
   }
 
   void
-  asyncPutData(const shared_ptr<const Data>& data)
+  asyncSend(const Block& wire)
   {
     this->ensureConnected(true);
-
-    lp::Packet packet;
-
-    shared_ptr<lp::CachePolicyTag> cachePolicyTag = data->getTag<lp::CachePolicyTag>();
-    if (cachePolicyTag != nullptr) {
-      packet.add<lp::CachePolicyField>(*cachePolicyTag);
-    }
-
-    packet.add<lp::FragmentField>(std::make_pair(data->wireEncode().begin(),
-                                                 data->wireEncode().end()));
-
-    m_face.m_transport->send(packet.wireEncode());
-  }
-
-  void
-  asyncPutNack(shared_ptr<const lp::Nack> nack)
-  {
-    this->ensureConnected(true);
-
-    lp::Packet packet;
-    packet.add<lp::NackField>(nack->getHeader());
-
-    Block interest = nack->getInterest().wireEncode();
-    packet.add<lp::FragmentField>(std::make_pair(interest.begin(), interest.end()));
-
-    m_face.m_transport->send(packet.wireEncode());
+    m_face.m_transport->send(wire);
   }
 
 public: // prefix registration

src/face.cpp

diff --git a/src/face.cpp b/src/face.cpp
index 537f1b2..fbaee26 100644
--- a/src/face.cpp
+++ b/src/face.cpp
@@ -238,33 +238,39 @@
 void
 Face::put(const Data& data)
 {
-  // Use original `data`, since wire format should already exist for the original Data
-  if (data.wireEncode().size() > MAX_NDN_PACKET_SIZE)
+  Block wire = data.wireEncode();
+
+  shared_ptr<lp::CachePolicyTag> cachePolicyTag = data.getTag<lp::CachePolicyTag>();
+  if (cachePolicyTag != nullptr) {
+    lp::Packet packet;
+    packet.add<lp::CachePolicyField>(*cachePolicyTag);
+    packet.add<lp::FragmentField>(std::make_pair(wire.begin(), wire.end()));
+    wire = packet.wireEncode();
+  }
+
+  if (wire.size() > MAX_NDN_PACKET_SIZE)
     BOOST_THROW_EXCEPTION(Error("Data size exceeds maximum limit"));
 
-  shared_ptr<const Data> dataPtr;
-  try {
-    dataPtr = data.shared_from_this();
-  }
-  catch (const bad_weak_ptr& e) {
-    std::cerr << "Face::put WARNING: the supplied Data should be created using make_shared<Data>()"
-              << std::endl;
-    dataPtr = make_shared<Data>(data);
-  }
-
-  // If the same ioService thread, dispatch directly calls the method
   IO_CAPTURE_WEAK_IMPL(dispatch) {
-    impl->asyncPutData(dataPtr);
+    impl->asyncSend(wire);
   } IO_CAPTURE_WEAK_IMPL_END
 }
 
 void
 Face::put(const lp::Nack& nack)
 {
-  auto nackPtr = make_shared<lp::Nack>(nack);
+  lp::Packet packet;
+  packet.add<lp::NackField>(nack.getHeader());
+  const Block& interestWire = nack.getInterest().wireEncode();
+  packet.add<lp::FragmentField>(std::make_pair(interestWire.begin(), interestWire.end()));
+
+  Block wire = packet.wireEncode();
+
+  if (wire.size() > MAX_NDN_PACKET_SIZE)
+    BOOST_THROW_EXCEPTION(Error("Nack size exceeds maximum limit"));
 
   IO_CAPTURE_WEAK_IMPL(dispatch) {
-    impl->asyncPutNack(nackPtr);
+    impl->asyncSend(wire);
   } IO_CAPTURE_WEAK_IMPL_END
 }
 

tests/unit-tests/face.t.cpp

diff --git a/tests/unit-tests/face.t.cpp b/tests/unit-tests/face.t.cpp
index 42def58..6e81644 100644
--- a/tests/unit-tests/face.t.cpp
+++ b/tests/unit-tests/face.t.cpp
@@ -304,13 +304,32 @@
 
 BOOST_AUTO_TEST_SUITE(Producer)
 
+BOOST_AUTO_TEST_CASE(PutData)
+{
+  BOOST_CHECK_EQUAL(face.sentData.size(), 0);
+
+  Data data("/4g7xxcuEow/KFvK5Kf2m");
+  signData(data);
+  face.put(data);
+
+  lp::CachePolicy cachePolicy;
+  cachePolicy.setPolicy(lp::CachePolicyType::NO_CACHE);
+  data.setTag(make_shared<lp::CachePolicyTag>(cachePolicy));
+  face.put(data);
+
+  advanceClocks(time::milliseconds(10));
+  BOOST_REQUIRE_EQUAL(face.sentData.size(), 2);
+  BOOST_CHECK(face.sentData[0].getTag<lp::CachePolicyTag>() == nullptr);
+  BOOST_CHECK(face.sentData[1].getTag<lp::CachePolicyTag>() != nullptr);
+}
+
 BOOST_AUTO_TEST_CASE(PutNack)
 {
   BOOST_CHECK_EQUAL(face.sentNacks.size(), 0);
 
   face.put(makeNack(Interest("/Hello/World", time::milliseconds(50)), lp::NackReason::NO_ROUTE));
-  advanceClocks(time::milliseconds(10));
 
+  advanceClocks(time::milliseconds(10));
   BOOST_CHECK_EQUAL(face.sentNacks.size(), 1);
 }