On Tue, Feb 16, 2010 at 2:47 PM, Romain Francois < romain.francois.r.enthusi...@gmail.com> wrote:
> Thanks for the feedback. > > > On 02/16/2010 10:44 PM, Kenton Varda wrote: > >> GeneratedMessageReflection is an internal class used by the protobuf >> implementation. Currently, users are not allowed to use it directly, >> because we may change it at any time. You're suggesting that we promote >> it to a public interface, which has implications with regard to >> maintenance costs and implementation agility. >> > > I understand that. > > I'm not necessarily suggesting to make this particular class public, but am > looking for some way to iterate over the elements of a repeated field. > The Reflection interface already provides a way -- FieldSize() and GetRepeatedX(). The only problem is that it's a bit slower than the generated accessors because these methods aren't inlineable. BTW, have you observed an actual performance problem or are you just speculating that this performance difference may be a problem for you? > On the same score, if I want to increase the number of elements in a > repeated field, I have to do it one by one right ? I can't do something like > first reserve space, and then fill the generated space. Does that mean that > memory is reallocated each time ? No, adding an element is O(1), even if you haven't reserved space. It works the same way std::vector works. > > What you would really need to >> use is dynamic_cast. My golden rule of dynamic_cast is that it should >> only be used for optimization, and you must provide an implementation >> for the case where dynamic_cast always returns NULL. >> > > But dynamic_cast is a runtime thing, where TMP dispatch happens at compile > time. > GetReflection() returns Reflection*, NOT GeneratedMessageReflection*. Therefore the compile has no idea which type was returned, therefore you *must* use a runtime check. That's my point -- your template dispatch doesn't accomplish anything in this context. > > In your case, you >> are doing this, so that should be fine. >> >> On Sat, Feb 13, 2010 at 1:16 AM, Romain Francois >> <romain.francois.r.enthusi...@gmail.com >> <mailto:romain.francois.r.enthusi...@gmail.com>> wrote: >> >> Hello, >> >> Thanks for the answers. >> >> Maybe I should give some more background on why this is of interest >> to me. In RProtoBuf, we essentially only use the reflection api so >> that we can dynamically load new proto message types at runtime, etc >> ... we don't use protoc and therefore have no access to the >> generated classes. >> >> In the class "GeneratedMessageReflection", there are templates such as >> : >> >> template <typename Type> >> inline Type GetRepeatedField(const Message& message, >> const FieldDescriptor* field, >> int index) const; >> >> but they are private ? Why ? >> >> >From what I can read of the code, methods like GetRepeatedInt32 >> get expanded out of : >> >> PASSTYPE GeneratedMessageReflection::GetRepeated##TYPENAME( >> \ >> const Message& message, \ >> const FieldDescriptor* field, int index) const { \ >> USAGE_CHECK_ALL(GetRepeated##TYPENAME, REPEATED, CPPTYPE); \ >> if (field->is_extension()) { \ >> return GetExtensionSet(message).GetRepeated##TYPENAME( \ >> field->number(), index); \ >> } else { \ >> return GetRepeatedField<TYPE>(message, field, index); \ >> } \ >> } \ >> >> so doing things like this code >> >> >> for( int i=0; i<size; i++){ >> INTEGER(res)[i] = (int) ref->GetRepeatedInt32( >> *message, >> fieldDesc, >> i ) ; >> } >> >> is going to be not as efficient as if I could directly iterate over >> the repeated field using RepeatedField::iterator >> >> Instead of extending the Reflection interface, what about making the >> templates public in GeneratedMessageReflection and then maybe use >> some sort of trait to indicate whether that the instance of >> Reflection I have access to has these templates. >> >> >> Something like : >> >> >> template <typename _T, _T _V> struct integral_constant { >> static const _T value = _V; >> typedef _T value_type; >> typedef integral_constant<_T,_V> type; >> }; >> typedef integral_constant<bool, true> true_type; >> typedef integral_constant<bool, false> false_type; >> >> struct reflection_has_templates{} ; >> struct reflection_no_templates{} ; >> >> template <typename T> struct has_templates : public false_type{}; >> template<> struct has_templates<GeneratedMessageReflection> : public >> true_type{} ; >> >> So that using this trait, I can dispatch between : >> - suboptimal implementation when the templates are not there >> - optimal implementation using RepeatedField::iterator when they are >> available. >> >> It adds nothing to the Reflection interface. >> >> Romain >> >> >> >> On 02/12/2010 10:37 PM, Kenton Varda wrote: >> >> Yeah, it would add a lot of complication and reduce the >> flexibility of >> the Reflection interface. We don't want to require repeated >> fields to >> be implemented in terms of RepeatedField or RepeatedPtrField. >> >> On Fri, Feb 12, 2010 at 12:11 PM, Jason Hsueh <jas...@google.com >> <mailto:jas...@google.com> >> <mailto:jas...@google.com <mailto:jas...@google.com>>> wrote: >> >> +kenton >> >> Kenton may have a better answer, but I surmise that it's to >> avoid >> tying the Reflection interface to implementation details. A >> Message >> implementation might not use RepeatedField at all. The original >> version of protobufs used a different class to represent >> repeated >> fields, so it wouldn't have been possible to implement >> Reflection >> for the original version if the interface required access to >> RepeatedField. So maybe the reason is historical. Recent >> changes >> have been made that would make this technically possible. >> However, >> adding methods to get direct access to the RepeatedField >> would still >> expand the Reflection interface quite a bit. I'll defer to >> Kenton on >> that. >> >> On Fri, Feb 12, 2010 at 2:51 AM, Romain Francois >> <romain.francois.r.enthusi...@gmail.com >> <mailto:romain.francois.r.enthusi...@gmail.com> >> <mailto:romain.francois.r.enthusi...@gmail.com >> <mailto:romain.francois.r.enthusi...@gmail.com>>> wrote: >> >> >> Why not ? It seems reasonnable to want to use e.g. >> std::copy and >> friends. >> >> On the documentation it says : >> >> " >> Most users will not ever use a RepeatedField directly; >> they will >> use the get-by-index, set-by-index, and add accessors >> that are >> generated for all repeated fields >> " >> >> What if I do want to use RepeatedField ? >> >> Romain >> >> >> On 02/11/2010 06:50 PM, Jason Hsueh wrote: >> >> No, there isn't a way to get the RepeatedField from the >> reflection >> interface. You can only do so via the generated >> interface. >> >> On Thu, Feb 11, 2010 at 5:57 AM, Romain Francois >> <romain.francois.r.enthusi...@gmail.com >> <mailto:romain.francois.r.enthusi...@gmail.com> >> <mailto:romain.francois.r.enthusi...@gmail.com >> <mailto:romain.francois.r.enthusi...@gmail.com>> >> <mailto:romain.francois.r.enthusi...@gmail.com >> <mailto:romain.francois.r.enthusi...@gmail.com> >> <mailto:romain.francois.r.enthusi...@gmail.com >> <mailto:romain.francois.r.enthusi...@gmail.com>>>> wrote: >> >> Hello, >> >> How can I get hold of a RepeatedField object to >> manage a >> repeated >> field in C++. >> >> In RProtoBuf, we do a lot of : >> >> for( int i=0; i<size; i++){ >> INTEGER(res)[i] = (int) ref->GetRepeatedInt32( >> *message, >> fieldDesc, >> i ) ; >> } >> >> where essentially the INTEGER macro gives a >> pointer to >> the beginning >> of the int array we are filling. >> >> I'd like to replace this using e.g std::copy >> >> RepeatedField field ; >> std::copy( field.begin(), field.end(), >> INTEGER(res) ) ; >> >> but I can't find how to actually get hold of a >> RepeatedField object. >> >> Is it possible ? >> >> Romain >> >> >> -- >> Romain Francois >> Professional R Enthusiast >> +33(0) 6 28 91 30 30 >> http://romainfrancois.blog.free.fr >> |- http://tr.im/NrTG : Rcpp 0.7.5 >> |- http://tr.im/MPYc : RProtoBuf: protocol buffers for R >> `- http://tr.im/KfKn : Rcpp 0.7.2 >> >> >> > > -- > Romain Francois > Professional R Enthusiast > +33(0) 6 28 91 30 30 > http://romainfrancois.blog.free.fr > |- http://tr.im/OcQe : Rcpp 0.7.7 > |- http://tr.im/O1wO : highlight 0.1-5 > `- http://tr.im/O1qJ : Rcpp 0.7.6 > > -- You received this message because you are subscribed to the Google Groups "Protocol Buffers" group. To post to this group, send email to proto...@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.
