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.

Sign up for GitHub

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

Jump to bottom

Java Proto lite: avoid boxing Integers accessing enum lists #16607

Closed
wants to merge 1 commit into from
Closed

Java Proto lite: avoid boxing Integers accessing enum lists #16607

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
32 changes: 32 additions & 0 deletions java/core/src/main/java/com/google/protobuf/CodedOutputStreamWriter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -557,6 +557,38 @@ private void writeDoubleListInternal(int fieldNumber, List<Double> value, boolea
@Override
public void writeEnumList(int fieldNumber, List<Integer> value, boolean packed)
throws IOException {
if (value instanceof IntArrayList) {
writeEnumListInternal(fieldNumber, (IntArrayList) value, packed);
} else {
writeEnumListInternal(fieldNumber, value, packed);
}
}

private void writeEnumListInternal(int fieldNumber, IntArrayList value, boolean packed)
throws IOException {
if (packed) {
output.writeTag(fieldNumber, WIRETYPE_LENGTH_DELIMITED);

// Compute and write the length of the data.
int dataSize = 0;
for (int i = 0; i < value.size(); ++i) {
dataSize += CodedOutputStream.computeEnumSizeNoTag(value.getInt(i));
}
output.writeUInt32NoTag(dataSize);

// Write the data itself, without any tags.
for (int i = 0; i < value.size(); ++i) {
output.writeEnumNoTag(value.getInt(i));
}
} else {
for (int i = 0; i < value.size(); ++i) {
output.writeEnum(fieldNumber, value.getInt(i));
}
}
}

private void writeEnumListInternal(int fieldNumber, List<Integer> value, boolean packed)
throws IOException {
if (packed) {
output.writeTag(fieldNumber, WIRETYPE_LENGTH_DELIMITED);

Expand Down
30 changes: 30 additions & 0 deletions java/core/src/main/java/com/google/protobuf/Internal.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -371,6 +371,36 @@ static Object mergeMessage(Object destination, Object source) {
return ((MessageLite) destination).toBuilder().mergeFrom((MessageLite) source).buildPartial();
}

/**
* Provides an immutable view of {@code List<T>} around an {@code IntList}.
*
* <p>Protobuf internal. Used in protobuf generated code only.
*/
public static class IntListAdapter<T> extends AbstractList<T> {
/** Convert individual elements of the List from int to T. */
public interface IntConverter<T> {
T convert(int from);
}

private final IntList fromList;
private final IntConverter<T> converter;

public IntListAdapter(IntList fromList, IntConverter<T> converter) {
this.fromList = fromList;
this.converter = converter;
}

@Override
public T get(int index) {
return converter.convert(fromList.getInt(index));
}

@Override
public int size() {
return fromList.size();
}
}

/**
* Provides an immutable view of {@code List<T>} around a {@code List<F>}.
*
Expand Down
4 changes: 4 additions & 0 deletions python/google/protobuf/internal/descriptor_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -884,6 +884,10 @@ def testCopyToProto_ForeignEnum(self):
name: 'FOREIGN_BAX'
number: 32
>
value: <
name: 'FOREIGN_LARGE'
number: 123456
>
"""

self._InternalTestCopyToProto(
Expand Down
30 changes: 24 additions & 6 deletions python/google/protobuf/internal/reflection_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -594,19 +594,37 @@ def testEnum_Value(self, message_module):

def testEnum_KeysAndValues(self, message_module):
if message_module == unittest_pb2:
keys = ['FOREIGN_FOO', 'FOREIGN_BAR', 'FOREIGN_BAZ', 'FOREIGN_BAX']
values = [4, 5, 6, 32]
keys = [
'FOREIGN_FOO',
'FOREIGN_BAR',
'FOREIGN_BAZ',
'FOREIGN_BAX',
'FOREIGN_LARGE',
]
values = [4, 5, 6, 32, 123456]
items = [
('FOREIGN_FOO', 4),
('FOREIGN_BAR', 5),
('FOREIGN_BAZ', 6),
('FOREIGN_BAX', 32),
('FOREIGN_LARGE', 123456),
]
else:
keys = ['FOREIGN_ZERO', 'FOREIGN_FOO', 'FOREIGN_BAR', 'FOREIGN_BAZ']
values = [0, 4, 5, 6]
items = [('FOREIGN_ZERO', 0), ('FOREIGN_FOO', 4),
('FOREIGN_BAR', 5), ('FOREIGN_BAZ', 6)]
keys = [
'FOREIGN_ZERO',
'FOREIGN_FOO',
'FOREIGN_BAR',
'FOREIGN_BAZ',
'FOREIGN_LARGE',
]
values = [0, 4, 5, 6, 123456]
items = [
('FOREIGN_ZERO', 0),
('FOREIGN_FOO', 4),
('FOREIGN_BAR', 5),
('FOREIGN_BAZ', 6),
('FOREIGN_LARGE', 123456),
]
self.assertEqual(keys,
list(message_module.ForeignEnum.keys()))
self.assertEqual(values,
Expand Down
4 changes: 2 additions & 2 deletions src/google/protobuf/compiler/cpp/unittest.inc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1022,8 +1022,8 @@ TEST(GENERATED_ENUM_TEST_NAME, MinAndMax) {
EXPECT_EQ(4, UNITTEST::TestAllTypes::NestedEnum_ARRAYSIZE);

EXPECT_EQ(UNITTEST::FOREIGN_FOO, UNITTEST::ForeignEnum_MIN);
EXPECT_EQ(UNITTEST::FOREIGN_BAX, UNITTEST::ForeignEnum_MAX);
EXPECT_EQ(33, UNITTEST::ForeignEnum_ARRAYSIZE);
EXPECT_EQ(UNITTEST::FOREIGN_LARGE, UNITTEST::ForeignEnum_MAX);
EXPECT_EQ(123457, UNITTEST::ForeignEnum_ARRAYSIZE);

EXPECT_EQ(1, UNITTEST::TestEnumWithDupValue_MIN);
EXPECT_EQ(3, UNITTEST::TestEnumWithDupValue_MAX);
Expand Down
25 changes: 12 additions & 13 deletions src/google/protobuf/compiler/java/lite/enum_field.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -597,27 +597,26 @@ void RepeatedImmutableEnumFieldLiteGenerator::GenerateMembers(
variables_,
"private com.google.protobuf.Internal.IntList $name$_;\n"
"private static final "
"com.google.protobuf.Internal.ListAdapter.Converter<\n"
" java.lang.Integer, $type$> $name$_converter_ =\n"
" new com.google.protobuf.Internal.ListAdapter.Converter<\n"
" java.lang.Integer, $type$>() {\n"
"com.google.protobuf.Internal.IntListAdapter.IntConverter<\n"
" $type$> $name$_converter_ =\n"
" new com.google.protobuf.Internal.IntListAdapter.IntConverter<\n"
" $type$>() {\n"
" @java.lang.Override\n"
" public $type$ convert(java.lang.Integer from) {\n"
" public $type$ convert(int from) {\n"
" $type$ result = $type$.forNumber(from);\n"
" return result == null ? $unknown$ : result;\n"
" }\n"
" };\n");
PrintExtraFieldInfo(variables_, printer);
WriteFieldAccessorDocComment(printer, descriptor_, LIST_GETTER,
context_->options());
printer->Print(
variables_,
"@java.lang.Override\n"
"$deprecation$public java.util.List<$type$> "
"${$get$capitalized_name$List$}$() {\n"
" return new com.google.protobuf.Internal.ListAdapter<\n"
" java.lang.Integer, $type$>($name$_, $name$_converter_);\n"
"}\n");
printer->Print(variables_,
"@java.lang.Override\n"
"$deprecation$public java.util.List<$type$> "
"${$get$capitalized_name$List$}$() {\n"
" return new com.google.protobuf.Internal.IntListAdapter<\n"
" $type$>($name$_, $name$_converter_);\n"
"}\n");
printer->Annotate("{", "}", descriptor_);
WriteFieldAccessorDocComment(printer, descriptor_, LIST_COUNT,
context_->options());
Expand Down
1 change: 1 addition & 0 deletions src/google/protobuf/unittest.proto
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,7 @@ enum ForeignEnum {
FOREIGN_BAR = 5;
FOREIGN_BAZ = 6;
FOREIGN_BAX = 32; // (1 << 32) to generate a 64b bitmask would be incorrect.
FOREIGN_LARGE = 123456; // Large enough to escape the Boxed Integer cache.
}

message TestReservedFields {
Expand Down
3 changes: 2 additions & 1 deletion src/google/protobuf/unittest_proto3.proto
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ message TestAllTypes {
repeated string repeated_string_piece = 54 [ctype = STRING_PIECE];
repeated string repeated_cord = 55 [ctype = CORD];

repeated NestedMessage repeated_lazy_message = 57 ;
repeated NestedMessage repeated_lazy_message = 57;

oneof oneof_field {
uint32 oneof_uint32 = 111;
Expand Down Expand Up @@ -178,6 +178,7 @@ enum ForeignEnum {
FOREIGN_FOO = 4;
FOREIGN_BAR = 5;
FOREIGN_BAZ = 6;
FOREIGN_LARGE = 123456;
}

// TestEmptyMessage is used to test behavior of unknown fields.
Expand Down
1 change: 1 addition & 0 deletions src/google/protobuf/unittest_proto3_arena.proto
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,7 @@ enum ForeignEnum {
FOREIGN_FOO = 4;
FOREIGN_BAR = 5;
FOREIGN_BAZ = 6;
FOREIGN_LARGE = 123456;
}

// TestEmptyMessage is used to test behavior of unknown fields.
Expand Down