On Thu, Dec 18, 2008 at 4:06 AM, Alek Storm <alek.st...@gmail.com> wrote:

> On Sat, Dec 13, 2008 at 5:09 PM, Petar Petrov <pesho.pet...@gmail.com>wrote:
>
>> On Mon, Dec 8, 2008 at 5:36 PM, Alek Storm <alek.st...@gmail.com> wrote:
>>
>>> Okay, then we just need to cache the size only during serialization.  The
>>> children's sizes are calculated and stored, then added to the parent's
>>> size.  Write the parent size, then write the parent, then the child size,
>>> then the child, on down the tree.  Then it's O(n) (same as we have
>>> currently) and no ownership problems, because we can drop the weak reference
>>> from child to parent.  Would that work?
>>
>>
>> It may work, but ByteSize is a part of the public interface of the
>> message, so making it slower may not be a good idea.
>> However the parent reference will still be needed.
>>
>> file.py:
>> m3 = M3()
>> m3.m2.m1.i = 3
>> m3.HasField('m2') # should be True
>>
>> How does m3 know if m2 was set? This information is right now provided by
>> the setter of 'i' in m1 (by calling TransitionToNonEmpty on the parent,
>> which calls TransitionToNonEmpty on its parent and so on).
>>
>
> Oops, I wasn't clear.  Of course HasField should work for non-repeated
> fields; I only meant to get rid of the weak reference when the message's
> parent is a repeated composite field, because HasField isn't used for those,
> so we don't need it if we cache the size only during serialization.  So we
> get a bunch of benefits in exchange for making a rarely used part of the
> interface slower, and only when used outside of the internal serialization
> functions.  What do you think?
>

I think it's a better idea to avoid adding features, which weren't
requested, until we have a solution to the real problem of the API - speed.
A change like this will make that problem even worse.
Extending the interface might make performance improvements harder to do.
While the API is slow features just fade out.


>
>
>> So the parent references are still needed. Let's keep the slice assignment
>> of repeated scalar fields and just remove the slice assignment of repeated
>> composite fields (I still don't find it usefull). E.g. we can keep
>> __getslice__, __delitem__ and __delslice__ for repeated composite fields.
>>
>
> Okay, I'll submit a patch with just those methods.  We can definitely agree
> on that :).  The above discussion is separate.


Looks like we are still keeping append, insert, remove and __setitem__?
Another concern here is that in an eventual C++ implementation of the API,
these will be a source of object ownership problems.
in cases like:
a = M1()
b = M1()
sub_message = a.add()
b.append(sub_message)
# who owns sub_message?

Adding them is a good idea, only when we have already solved the performance
problem (not before).


>
> By the way, I think something went wrong with your email - apparently it
> was sent to the group, but didn't show up there, so I just now found it in
> my inbox.  Weird.
>
> Cheers,
> Alek Storm
>

--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups 
"Protocol Buffers" group.
To post to this group, send email to protobuf@googlegroups.com
To unsubscribe from this group, send email to 
protobuf+unsubscr...@googlegroups.com
For more options, visit this group at 
http://groups.google.com/group/protobuf?hl=en
-~----------~----~----~----~------~----~------~--~---

Reply via email to