Re: All Bonobo patches must be approved from now on.




Elliot Lee writes:
 > This is the way the OMG spec does it. This is the way everyone else has
 > done it in the past. Calling it "totally unacceptable" is a bit
 > extreme.

    Ok, I'd rather do it the other way in Bonobo, setting each epv
method manually.

 > >            To make matters worse, this was done very inconsistently,
 > >            with the epv kept in a static function variable half the
 > >            time and put in a publicly-exported epv variable in other
 > >            places.
 > 
 > Well, that is not hard to fix.

    Ok, good point.

 > >            Furthermore, we've decided that it really isn't worth it to
 > >            complicate the epv-filling functions just to save 1k of
 > >            RAM.  It's in the noise and makes Bonobo more complicated.
 > >            Bonobo is supposed to be the paragon of elegance and
 > >            cleanliness, and I'm not willing to sacrifice that for a
 > >            few bytes of savings.
 > 
 > There is no elegance in removing features. Sure, people who are
 > implementing new objects will have to learn what the 'duplicate' parameter
 > is, but there is so much complexity involved already with implementing an
 > object that learning what that parameter does is the least of their
 > worries.

    I'd rather keep the complexity as low as possible.  


 > Too many times people have said "a few K of additional memory usage
 > doesn't matter". The problem with saying this is that people say it over
 > and over, and while one single occurrance of a wasting a few K *doesn't*
 > matter, they all add up. Creeping mediocrity is our worst enemy, and this
 > is a classic example of it.

    I wish I could agree but in this case the space savings are so
small, it seems really silly.  Each GnomeObject epv is 16 bytes, and
there are about 40 classes (max), so we're talking about 640 bytes.
This seems really silly.  

    Next you're going to be arguing to remove preconditions because
each one uses 16 bytes.  I mean, take a look at this huge waste of
core:

	g_return_val_if_fail (GNOME_IS_MONIKER (moniker), NULL);
 1e2:	83 3e 00             	cmpl   $0x0,(%esi)
 1e5:	74 16                	je     1fd <gnome_moniker_get_as_string+0x39>
 1e7:	e8 04 02 00 00       	call   3f0 <gnome_moniker_get_type>
 1ec:	50                   	pushl  %eax
 1ed:	8b 06                	movl   (%esi),%eax
 1ef:	ff 30                	pushl  (%eax)
 1f1:	e8 fc ff ff ff       	call   1f2 <gnome_moniker_get_as_string+0x2e>
 1f6:	83 c4 08             	addl   $0x8,%esp
 1f9:	85 c0                	testl  %eax,%eax
 1fb:	75 29                	jne    226 <gnome_moniker_get_as_string+0x62>
 1fd:	68 75 00 00 00       	pushl  $0x75
 202:	68 f1 00 00 00       	pushl  $0xf1
 207:	6a 66                	pushl  $0x66
 209:	68 29 00 00 00       	pushl  $0x29
 20e:	68 40 00 00 00       	pushl  $0x40
 213:	6a 08                	pushl  $0x8
 215:	68 6e 00 00 00       	pushl  $0x6e
 21a:	e8 fc ff ff ff       	call   21b <gnome_moniker_get_as_string+0x57>
 21f:	31 c0                	xorl   %eax,%eax
 221:	e9 18 01 00 00       	jmp    33e <gnome_moniker_get_as_string+0x17a>

And that's just one precondition!  63 bytes!  I count 1326 such
preconditions in Bonobo.  The horror!

 > A bigger issue is that what you are trying to accomplish (implementation
 > inheritance) is a totally bad hack not supported nor recommended by the
 > CORBA spec at all (it may be more easily possible with the C++ mappings,
 > but definitely not the C mappings). I know you will not want to admit that
 > it is not a bad hack, because you are trying to implement something that
 > depends on this feature.

    I don't understand what's bad about this.  Why does the vepv exist 
if not to allow you to use impl inheritance?

 > >         3. Elliot added ifdefs everywhere to support OAF.  How to
 > >            support Oaf is still an open issue in Bonobo, so this was
 > >            inappropriate.  Furthermore, ifdefs like this are ugly and
 > >            make the code harder to maintain and read.  This will never 
 > >            go in.  
 > 
 > Yup.

    I'd rather see liboaf released and then we can make Bonobo depend
on it.

 > Can we just put the goad_id after the closure parameter, so that these
 > updates aren't needed, but the needed functionality goes in.

    Well, we currently require that you register a factory for each
GOAD ID you're going to support.  So why do we need the parameter?
Does it make sense for us to change things so that one factory can
produce objects with many different goad ids?

 > Furthermore, the "sanity check" was preventing some valid cases of
 > aggregation, and would not have prevented some "invalid" (your definition)
 > cases. There is no way at all to tell whether an object has been exported
 > to the world - you have no control over whether the objref has gone to
 > China and back, and the refcount may have changed due to internal
 > ownership indications rather than usage by an external client.

    I'm actually starting to think you may be right.  It's probably
better to just *ask* people to adhere to the maxim than to enforce it
with code in this case.

Nat



[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]