Re: Implement ShutdownProtobufLibrary().

2009-05-06 Thread Kenton Varda
Thanks, committed as revision 138. On Wed, May 6, 2009 at 12:26 PM, wrote: > Looks good! > > > On 2009/05/06 19:05:36, kenton wrote: > >> Addressed comments (see below) and synced, new patch set uploaded. >> > > http://codereview.appspot.com/53053/diff/1/5 >> File google/protobuf/compiler/cpp/c

Re: Implement ShutdownProtobufLibrary().

2009-05-06 Thread jasonh
Looks good! On 2009/05/06 19:05:36, kenton wrote: > Addressed comments (see below) and synced, new patch set uploaded. > http://codereview.appspot.com/53053/diff/1/5 > File google/protobuf/compiler/cpp/cpp_file.cc (right): > http://codereview.appspot.com/53053/diff/1/5#newcode401 > Line 401: >

Re: Implement ShutdownProtobufLibrary().

2009-05-06 Thread kenton
Addressed comments (see below) and synced, new patch set uploaded. http://codereview.appspot.com/53053/diff/1/5 File google/protobuf/compiler/cpp/cpp_file.cc (right): http://codereview.appspot.com/53053/diff/1/5#newcode401 Line 401: On 2009/05/06 01:03:14, jasonh wrote: > Indent() for prettier

Re: Implement ShutdownProtobufLibrary().

2009-05-06 Thread Kenton Varda
On Tue, May 5, 2009 at 8:15 PM, Hong Zhang wrote: > Hi, Kenton, > > I am not familiar with protobuf compiler, so my comment may be off. > > Since existing code does something like this. > > static class Foo { > Foo() { ... } > } foo; Actually, a lot of code doesn't look like that anymore, beca

Re: Implement ShutdownProtobufLibrary().

2009-05-05 Thread Hong Zhang
By default, xxx will be false. So the overhead is some some global variable checks and nothing gets done. Most application only uses several .proto files, so the overhead is close to 0. The global variable xxx will be defined inside base protobuf library. Hong --~--~-~--~~---

Re: Implement ShutdownProtobufLibrary().

2009-05-05 Thread Jason Hsueh
I believe the reasoning against this was for the case where the client doesn't care whether the memory is freed - in particular, if the shutdown is happening right before the process exits, then it would be doing a bunch of work to free memory that's going back to the OS anyway. Apparently this can

Re: Implement ShutdownProtobufLibrary().

2009-05-05 Thread Hong Zhang
Hi, Kenton, I am not familiar with protobuf compiler, so my comment may be off. Since existing code does something like this. static class Foo { Foo() { ... } } foo; What you can do it like this static class Foo { Foo(); ~Foo(); } foo; Inside ~Foo(), you can do a check if (xxx), where

Re: Implement ShutdownProtobufLibrary().

2009-05-05 Thread jasonh
Sorry about the delay. Mostly looks good, just a few nits http://codereview.appspot.com/53053/diff/1/5 File google/protobuf/compiler/cpp/cpp_file.cc (right): http://codereview.appspot.com/53053/diff/1/5#newcode401 Line 401: Indent() for prettier generated code? http://codereview.appspot.com/53