Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Sync nanoproto from AOSP #315

Merged
merged 13 commits into from Apr 29, 2015
Merged

Conversation

brianduff
Copy link
Contributor

No description provided.

brianduff and others added 13 commits April 28, 2015 11:42
Previously, extensions with field numbers greater than 268435455 would
result in a compile time error in generated code that looks something
like this:

Foo.java:3178: error: integer number too large: 3346754610
                3346754610);

This is because we were trying to represent the tag number (an
unsigned int) using a java int constant, but java int constants are
signed, and can't exceed Integer.MAX_VALUE.

Fixed by declaring it as a long instead, and casting it down to an
int in the implementation. This is safe, because the tag value always
fits in 32 bis.

Change-Id: If2017bacb4e20af667eaeaf9b65ddc2c30a7709f
Measuring the serialized size of nano protos is now a zero-alloc operation, and serializing a proto now allocates no memory (other than the output buffer) instead of O(total length of strings).

Change-Id: Id5e2ac3bdc4ac56c0bf13d725472da3a00c9baec
Signed-off-by: Charles Munger <clm@google.com>
Forgot to update these in https://android-review.googlesource.com/#/c/109809/

Change-Id: I53f838e2f134f53964161d9620d5ead00c4a3939
https://android-review.googlesource.com/#/c/67890/ removed field
initialization from the ctor, making it just call clear() instead.

When I added the generate_clear option back (as part of the reftypes
compat mode) in https://android-review.googlesource.com/#/c/109530/,
I forgot to ensure that what clear() used to do was inlined in the
constructor.

This change fixes NPEs that are happening for users of
reftypes_compat_mode who rely on unset repeated fields being empty
arrays rather than null.

Change-Id: Idb58746c60f4a4054b7ebb5c3b0e76b16ff88184
Upstreamed from Another Place (cr/57247854).

Change-Id: I2aaf59544c0f5ae21a51891d8a5eeda1dc722c90
Change-Id: I85563b74237d38c1e447b7286f5f6e62d57e3d63
…eger.

When building, some instances expect createMessageTyped to have the signature
(int, Class, long), while others expect (int, Class, int). Simply having
the former signature meant that builds expecting the latter would fail.

This is a cherrypick of change b2a9d43
from master.

Change-Id: Ib02dbf66173510f4edea32c7b43e82c1a7a38aa2
Change-Id: Ie2a9e36276ac35e10b3f8d379b5742d50a0374e9
@IntDef is a support library annotation which allows build tools to
determine the valid set of values for a given integer field when that
field is intended to be restricted like an enum. This avoids the
overhead of enums while still allowing for compile-time type checking
in most circumstances.

Change-Id: Iee02e0b49a8e069f6456572f538e0a0d301fdfd5
I wasn't able to get the clear() method to inline into the
constructor when optimizations are on in proguard. As a result,
every message has an extra superfluous kept method assuming the
app never uses clear() directly.

There are a couple of instances where setting this option false is
necessary in order to get code dexing successfully without hitting
the method limit, e.g. https://goto.google.com/tltzq

In this example, I tried turning on the method/inlining/unique and
method/inlining/short optimizations before resorting to adding the
generate_clear option, but the method count did not decrease. The
clear() methods were contributing over a thousand extra methods.

Change-Id: If6a9651d6a59cdf70b1040d8248779710ac73105
It turns out dex (apparently) was inlining these protected final
methods from ExtendableMessageNano into every message class. Removing
these methods from the base class and inlining their code reduces
the method count by 2 methods / message when the store_unknown_fields
option is on.

Change-Id: I0aa09f2016d39939c4c8b8219601793b8fab301f
When a MessageNano containing a String is serialized into a buffer that
is too small to contain it, and the buffer's boundary happens to be
where the string field's length delimiting varint is serialized,
and the string's length and 3*length have the same length when
encoded as a varint, an IllegalArgumentException is thrown rather than
an OutOfSpaceException.

Github issue: protocolbuffers#292

Change-Id: If478d68cf15bfd0662252d008e42b2bf1ff1c75e
This is less ideal from a dex count perspective because it requires a
new variable for each message, and because most apps have proguard
rules that will ensure that CREATOR classes are retained.

However, it is required to be able to use nano protos inside of AIDL
files, as the autogenerated AIDL code fails to compile otherwise. This
is a substantial benefit as it allows for backwards-compatible
parameters and return types in AIDL methods along the lines of
safeparcel.

Bug: 19084705
Change-Id: I66a2c0424b96cf8ff6b631b186cc4f9407dfc1f4
@@ -97,19 +97,19 @@
<arg value="src/test/java/com/google/protobuf/nano/map_test.proto" />
</exec>
<exec executable="../src/protoc">
<arg value="--javanano_out=store_unknown_fields=true,generate_equals=true:target/generated-test-sources" />
<arg value="--javanano_out=store_unknown_fields=true,generate_equals=true,generate_clone=true:target/generated-test-sources" />
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to run the test without android runtime. How do we handle the issue internally?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops, I misread that with the intdef annotation, nvm.

@liujisi
Copy link
Contributor

liujisi commented Apr 28, 2015

LGTM

liujisi added a commit that referenced this pull request Apr 29, 2015
@liujisi liujisi merged commit 0e122ce into protocolbuffers:master Apr 29, 2015
taoso pushed a commit to taoso/protobuf that referenced this pull request Aug 1, 2018
Found with github.com/mdempsky/unconvert.

	/Users/tamird/src/go/src/github.com/golang/protobuf/jsonpb/jsonpb.go:625:32: unnecessary conversion
	/Users/tamird/src/go/src/github.com/golang/protobuf/proto/encode.go:177:30: unnecessary conversion
	/Users/tamird/src/go/src/github.com/golang/protobuf/proto/encode.go:181:26: unnecessary conversion
	/Users/tamird/src/go/src/github.com/golang/protobuf/proto/text_parser.go:868:21: unnecessary conversion
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants