Very Computer

Thanks for your reply. Please see my answers to your questions below.

Quote:

> > >>     SAMPLE FIX/WORKAROUND:
> > >> I tried replacing the call to operator<<(TAO_OutputCDR&,
> const Any&)
> > >> in TAO_CDR_Encaps_Codec::encode by the following:
> > >> ("data" is the CORBA_ANY, "cdr" is the temporary TAO_OutputCDR)

> > >>   CORBA::TypeCode_var tc = data.type ();
> > >>   cdr << tc.in();
> > >>   cdr.write_octet_array_mb(data._tao_get_cdr());

> Hmm.. Isnt the write_octet_array_mb () supposed to copy data?
> The latest code, atleast, seems to copy data and I am not sure
> what is the performance gain that you got?

In other words, the idea is: Since the Any already contains a CDR
representation of the data, why not blindly append it to the OutputCDR
instead of using perform_append()? Is it because of alignment issues?

Pushing further, why not directly modify the implementation of
operator<<(TAO_OutputCDR&, const Any&) to blindly append the
Any's cdr_ member data to the OutputCDR instead of using
TAO_Marshal_Object::perform_append()?

Quote:> > >> Basically, the idea is that, since we can directly get a CDR
> > >> representation of the Any's data with CORBA_Any::_tao_get_cdr(),
> > >> we could save the marshalling by simply appending this data to
> > >> the output stream. As the Any already contains the encoded data,
> > >> _tao_get_cdr() is a very cheap operation, and we get a big
> > >> performance gain.

> > >> In the few tests I made, the result (the octet sequence) is
> > >> exactly the same using both approaches. However, this
> seems "too good
> > >> to be true" (or "too simple to be right").

> Did you do a performance testing? IF you did get some
> performance gain,
> would it be possible for you to see what is the reason for gain?
> I dont think it is the single copy, but more copies involved during
> Any marshalling. Could you please help us with that? Fixing that
> would be very helpful since maintenance would be far more easier
> using the extraction operators instead of CDR methods that you and me
> understand.

//IDL...
struct TestStruct
{
    long ALong;
    string AString;
    wstring AWString;

Quote:};

interface Dummy
{
   void Test(in TestStructSeqSeq TheSeqSeq);

Quote:};

I create a "monster" TestStructSeqSeq containing 1000 TestStructSeq,
each of which containing 100 TestStruct. This is then inserted
into an Any, and the Any is then passed to TAO_CDR_Encaps_Codec::encode().

Using MSVC's profiler, I got an average of 1800ms for the call to
TAO_CDR_Encaps_Codec::encode() using the original implementation.
The vast majority of that time (more than 90%) is spent in
TAO_Marshal_Object::perform_append().

With the modified version, the profiler reports an average of
50ms for the call to TAO_CDR_Encaps_Codec::encode().

Thanks,
Eric

 > > > >>
 > > > >>   CORBA::TypeCode_var tc = data.type ();
 > > > >>   cdr << tc.in();
 > > > >>   cdr.write_octet_array_mb(data._tao_get_cdr());
 > >
 > > Hmm.. Isnt the write_octet_array_mb () supposed to copy data?
 > > The latest code, atleast, seems to copy data and I am not sure
 > > what is the performance gain that you got?
 > >
 >
 > Yes, write_octet_array_mb() copies the data. However,
 > it _only_ copies it instead of marshalling it like the
 > TAO_Marshal_Object::perform_append() call used in
 > operator<<(TAO_OutputCDR&, const Any&).
 >
 > In other words, the idea is: Since the Any already contains a CDR
 > representation of the data, why not blindly append it to the OutputCDR
 > instead of using perform_append()? Is it because of alignment issues?
 >
 > Pushing further, why not directly modify the implementation of
 > operator<<(TAO_OutputCDR&, const Any&) to blindly append the
 > Any's cdr_ member data to the OutputCDR instead of using
 > TAO_Marshal_Object::perform_append()?

I had to modify your idea a bit to take care of the byte order. Just
appending an octet from an Any with a differing byte order could
create problems during unmarshalling. Please use the attached patch
and let me know how it goes. I can get this into the upcoming beta if
possible.

Thanks & Regards
Bala

Index: Any.cpp
===================================================================
RCS file: /project/cvs-repository/ACE_wrappers-repository/TAO/tao/Any.cpp,v
retrieving revision 1.128
diff -u -r1.128 Any.cpp
--- Any.cpp     17 Jul 2002 04:58:15 -0000      1.128

       return 0;
     }

-  ACE_TRY_NEW_ENV
+  int byte_order =
+    cdr.do_byte_swap () ? !ACE_CDR_BYTE_ORDER : ACE_CDR_BYTE_ORDER;
+
+  if (byte_order == x._tao_byte_order ())
+    {
+      cdr.write_octet_array_mb (x._tao_get_cdr ());
+    }
+  else
     {
-      TAO_InputCDR input (x._tao_get_cdr (),
-                          x._tao_byte_order ());
+      ACE_TRY_NEW_ENV
+        {
+          TAO_InputCDR input (x._tao_get_cdr (),
+                              x._tao_byte_order ());

-      CORBA::TypeCode::traverse_status status =
-        TAO_Marshal_Object::perform_append (tc.in (),
-                                            &input,
-                                            &cdr
-                                            ACE_ENV_ARG_PARAMETER);
-      ACE_TRY_CHECK;
+          CORBA::TypeCode::traverse_status status =
+            TAO_Marshal_Object::perform_append (tc.in (),
+                                                &input,
+                                                &cdr
+                                                ACE_ENV_ARG_PARAMETER);
+          ACE_TRY_CHECK;

-      if (status != CORBA::TypeCode::TRAVERSE_CONTINUE)
+          if (status != CORBA::TypeCode::TRAVERSE_CONTINUE)
+            {
+              return 0;
+            }
+        }
+      ACE_CATCH (CORBA_Exception, ex)
         {
           return 0;
         }
+      ACE_ENDTRY;
     }
-  ACE_CATCH (CORBA_Exception, ex)
-    {
-      return 0;
-    }
-  ACE_ENDTRY;
   return 1;
 }

You're (of course) totally right about the byte order check. My
optimization was a bit too expeditive...

I ran my tests using the modifications below and everything looks fine.

Thanks a lot for your feedback!

Eric