Changed the 1.1 encoding to write pending objects only if necessary

2 files changed, 112 insertions, 60 deletions

diff --git a/cpp/include/Ice/BasicStream.h b/cpp/include/Ice/BasicStream.h
index 65533921b07..ebdd333fc86 100644
--- a/cpp/include/Ice/BasicStream.h
+++ b/cpp/include/Ice/BasicStream.h
@@ -152,6 +152,11 @@ public:
     {
         assert(_currentWriteEncaps);
 
+        if(_currentWriteEncaps->encoder)
+        {
+            _currentWriteEncaps->encoder->writePendingObjects();
+        }
+
         Container::size_type start = _currentWriteEncaps->start;
         Ice::Int sz = static_cast<Ice::Int>(b.size() - start); // Size includes size and version.
         Ice::Byte* dest = &(*(b.begin() + start));
@@ -246,6 +251,12 @@ public:
     void endReadEncaps()
     {
         assert(_currentReadEncaps);
+
+        if(_currentReadEncaps->decoder)
+        {
+            _currentReadEncaps->decoder->readPendingObjects();
+        }
+
         Container::size_type start = _currentReadEncaps->start;
         Ice::Int sz = _currentReadEncaps->sz;
         if(i != b.begin() + start + sz)
@@ -476,7 +487,6 @@ public:
     {
         if(i >= b.end())
         {
-            assert(false);
             throwUnmarshalOutOfBoundsException(__FILE__, __LINE__);
         }
         v = *i++;
@@ -730,7 +740,7 @@ private:
     public:
         EncapsDecoder(BasicStream* stream, ReadEncaps* encaps, bool sliceObjects) :
             _stream(stream), _encaps(encaps), _sliceObjects(sliceObjects), _traceSlicing(-1), _sliceType(NoSlice),
-            _typeIdIndex(0)
+            _usesClasses(false), _typeIdIndex(0)
         {
         } 
 
@@ -768,6 +778,7 @@ private:
         bool _skipFirstSlice;
         Ice::SliceInfoSeq _slices;          // Preserved slices.
         IndexListList _indirectionTables;   // Indirection tables for the preserved slices.
+        bool _usesClasses;
 
         // Slice attributes
         Ice::Byte _sliceFlags;
@@ -786,7 +797,8 @@ private:
     {
     public:
         EncapsEncoder(BasicStream* stream, WriteEncaps* encaps, Ice::FormatType format) : 
-            _stream(stream), _encaps(encaps), _format(format), _sliceType(NoSlice), _objectIdIndex(0), _typeIdIndex(0)
+            _stream(stream), _encaps(encaps), _format(format), _sliceType(NoSlice), _usesClasses(false),
+            _objectIdIndex(0), _typeIdIndex(0)
         {
         }
 
@@ -807,7 +819,6 @@ private:
     private:
 
         void writeTypeId(const std::string&);
-        void writeInstance(const Ice::ObjectPtr&, Ice::Int);
         void writeSlicedData(const Ice::SlicedDataPtr&);
         Ice::Int registerObject(const Ice::ObjectPtr&);
 
@@ -818,6 +829,8 @@ private:
         // Object/exception attributes
         SliceType _sliceType;
         bool _firstSlice;
+        bool _usesClasses;
+        Container::size_type _usesClassesPos;
 
         // Slice attributes
         Ice::Byte _sliceFlags;
diff --git a/cpp/src/Ice/BasicStream.cpp b/cpp/src/Ice/BasicStream.cpp
index a207e14b88e..5295c73ea35 100755
--- a/cpp/src/Ice/BasicStream.cpp
+++ b/cpp/src/Ice/BasicStream.cpp
@@ -1617,6 +1617,7 @@ IceInternal::BasicStream::EncapsEncoder::write(const ObjectPtr& v)
             // Object references are encoded as a negative integer in 1.0.
             //
             _stream->write(-index);
+            _usesClasses = true;
         }
         else if(_sliceType != NoSlice && _format == SlicedFormat)
         {
@@ -1657,6 +1658,7 @@ IceInternal::BasicStream::EncapsEncoder::write(const ObjectPtr& v)
         if(_encaps->encoding == Encoding_1_0)
         {
             _stream->write(0);
+            _usesClasses = true;
         }
         else
         {
@@ -1668,16 +1670,8 @@ IceInternal::BasicStream::EncapsEncoder::write(const ObjectPtr& v)
 void
 IceInternal::BasicStream::EncapsEncoder::write(const UserException& v)
 {
-    //
-    // TODO: Eliminate leading usesClasses flag?
-    //
-    const bool usesClasses = v.__usesClasses();
-    _stream->write(usesClasses);
     v.__write(_stream);
-    if(usesClasses)
-    {
-        writePendingObjects();
-    }
+    writePendingObjects();
 }
 
 void
@@ -1711,6 +1705,11 @@ IceInternal::BasicStream::EncapsEncoder::startException(const SlicedDataPtr& dat
 {
     _sliceType = ExceptionSlice;
     _firstSlice = true;
+    if(_encaps->encoding == Encoding_1_0)
+    {
+        _usesClassesPos = _stream->b.size();
+        _stream->write(false); // Placeholder for usesClasses boolean
+    }
     if(data)
     {
         writeSlicedData(data);
@@ -1720,6 +1719,11 @@ IceInternal::BasicStream::EncapsEncoder::startException(const SlicedDataPtr& dat
 void 
 IceInternal::BasicStream::EncapsEncoder::endException()
 {
+    if(_encaps->encoding == Encoding_1_0)
+    {
+        Byte* dest = &(*(_stream->b.begin() + _usesClassesPos));
+        *dest = static_cast<Byte>(_usesClasses);
+    }
     _sliceType = NoSlice;
 }
 
@@ -1850,6 +1854,25 @@ IceInternal::BasicStream::EncapsEncoder::endSlice()
 void
 IceInternal::BasicStream::EncapsEncoder::writePendingObjects()
 {
+    //
+    // With the 1.0 encoding, only write pending objects if nil or
+    // non-nil references were written (_usesClasses =
+    // true). Otherwise, write pending objects only if some non-nil
+    // references were written.
+    //
+    if(_encaps->encoding == Encoding_1_0)
+    {
+        if(!_usesClasses)
+        {
+            return;
+        }
+        _usesClasses = false;
+    }
+    else if(_toBeMarshaledMap.empty())
+    {
+        return;
+    }
+
     while(!_toBeMarshaledMap.empty())
     {
         //
@@ -1866,46 +1889,38 @@ IceInternal::BasicStream::EncapsEncoder::writePendingObjects()
         for(PtrToIndexMap::iterator p = savedMap.begin(); p != savedMap.end(); ++p)
         {
             //
-            // Add an instance from the old to-be-marshaled map to
-            // the marshaled map and then ask the instance to
-            // marshal itself. Any new class instances that are
-            // triggered by the classes marshaled are added to
-            // toBeMarshaledMap.
+            // Ask the instance to marshal itself. Any new class
+            // instances that are triggered by the classes marshaled
+            // are added to toBeMarshaledMap.
             //
-            writeInstance(p->first, p->second);
-        }
-    }
-    _stream->writeSize(0); // Zero marker indicates end of sequence of sequences of instances.
-}
+            if(_encaps->encoding == Encoding_1_0)
+            {
+                _stream->write(p->second);
+            }
+            else
+            {
+                _stream->writeSize(p->second);
+            }
 
-void
-IceInternal::BasicStream::EncapsEncoder::writeInstance(const ObjectPtr& v, Int index)
-{
-    if(_encaps->encoding == Encoding_1_0)
-    {
-        _stream->write(index);
-    }
-    else
-    {
-        _stream->writeSize(index);
-    }
+            try
+            {
+                p->first->ice_preMarshal();
+            }
+            catch(const std::exception& ex)
+            {
+                Warning out(_stream->instance()->initializationData().logger);
+                out << "std::exception raised by ice_preMarshal:\n" << ex;
+            }
+            catch(...)
+            {
+                Warning out(_stream->instance()->initializationData().logger);
+                out << "unknown exception raised by ice_preMarshal";
+            }
 
-    try
-    {
-        v->ice_preMarshal();
-    }
-    catch(const std::exception& ex)
-    {
-        Warning out(_stream->instance()->initializationData().logger);
-        out << "std::exception raised by ice_preMarshal:\n" << ex;
-    }
-    catch(...)
-    {
-        Warning out(_stream->instance()->initializationData().logger);
-        out << "unknown exception raised by ice_preMarshal";
+            p->first->__write(_stream);
+        }
     }
-
-    v->__write(_stream);
+    _stream->writeSize(0); // Zero marker indicates end of sequence of sequences of instances.
 }
 
 void
@@ -1988,6 +2003,7 @@ IceInternal::BasicStream::EncapsDecoder::read(PatchFunc patchFunc, void* patchAd
         // Object references are encoded as a negative integer in 1.0.
         //
         _stream->read(index);
+        _usesClasses = true;
         if(index > 0)
         {
             throw MarshalException(__FILE__, __LINE__, "invalid object id");
@@ -2039,9 +2055,20 @@ IceInternal::BasicStream::EncapsDecoder::throwException(const UserExceptionFacto
 {
     assert(_sliceType == NoSlice);
 
-    // TODO: Get rid of this for 1.1?
-    bool usesClasses;
-    _stream->read(usesClasses);
+    if(_encaps->encoding == Encoding_1_0)
+    {
+        //
+        // User exception with the 1.0 encoding start with a boolean
+        // flag that indicates whether or not the exception has
+        // classes. This allows reading the pending objects even if
+        // some the exception was sliced. With encoding > 1.0, we
+        // don't need this, each slice indirect patch table indicates
+        // the presence of objects.
+        //
+        bool usesClasses;
+        _stream->read(usesClasses);
+        _usesClasses |= usesClasses;
+    }
 
     _sliceType = ExceptionSlice;
     _skipFirstSlice = false;
@@ -2080,14 +2107,8 @@ IceInternal::BasicStream::EncapsDecoder::throwException(const UserExceptionFacto
             catch(UserException& ex)
             {
                 ex.__read(_stream);
-                ex.__usesClasses(usesClasses);
-        
-                if(usesClasses)
-                {
-                    readPendingObjects();
-                }
-        
-                ex.ice_throw();
+                readPendingObjects();
+                throw;
         
                 // Never reached.
             }
@@ -2342,6 +2363,24 @@ IceInternal::BasicStream::EncapsDecoder::endSlice()
 void
 IceInternal::BasicStream::EncapsDecoder::readPendingObjects()
 {
+    //
+    // With the 1.0 encoding, only read pending objects if nil or
+    // non-nil references were read (_usesClasses == true). Otherwise,
+    // read pending objects only if some non-nil references were read.
+    //
+    if(_encaps->encoding == Encoding_1_0)
+    {        
+        if(!_usesClasses)
+        {
+            return;
+        }
+        _usesClasses = false;
+    }
+    else if(_patchMap.empty())
+    {
+        return;
+    }
+
     Int num;
     ObjectList objectList;
     do