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 -~----------~----~----~----~------~----~------~--~---