Improve performance of parsing unknown fields in Java#9371
Merged
acozzette merged 1 commit intoprotocolbuffers:masterfrom Jan 5, 2022
Merged
Improve performance of parsing unknown fields in Java#9371acozzette merged 1 commit intoprotocolbuffers:masterfrom
acozzette merged 1 commit intoprotocolbuffers:masterfrom
Conversation
elharo
reviewed
Jan 5, 2022
Contributor
elharo
left a comment
elharo
left a comment
There was a problem hiding this comment.
Not sure why the tests are failing. Has CI gone flaky again?
| @@ -0,0 +1,78 @@ | |||
| // Protocol Buffers - Google's data interchange format | |||
| // Copyright 2008 Google Inc. All rights reserved. | |||
Contributor
There was a problem hiding this comment.
2021 and otherwise update to current guidelines for copyright statements
Credit should go to @elharo for most of these Java changes--I am just cherry-picking them from our internal codebase. The one thing I did change was to give the UTF-8 validation tests their own Bazel test target. This makes it possible to give the other tests a shorter timeout, which is important for UnknownFieldSetPerformanceTest in particular.
elharo
reviewed
Jan 5, 2022
Contributor
elharo
left a comment
elharo
left a comment
There was a problem hiding this comment.
Test failures look related:
++ '[' '!' -z 'java/core/src/test/java/com/google/protobuf/UnknownFieldSetPerformanceTest.java ' ']'
++ echo 'Missing files in EXTRA_DIST: java/core/src/test/java/com/google/protobuf/UnknownFieldSetPerformanceTest.java '
Missing files in EXTRA_DIST: java/core/src/test/java/com/google/protobuf/UnknownFieldSetPerformanceTest.java
++ exit 1
acozzette
force-pushed
the
java-unknown-fields
branch
from
624452b to
5ea2bdf
Compare
Author
|
#9372 should fix the Ruby build failure if I understand the problem right. |
elharo
approved these changes
Jan 5, 2022
| @@ -232,6 +232,7 @@ | |||
| <exclude>TypeRegistryTest.java</exclude> | |||
| <exclude>UnknownEnumValueTest.java</exclude> | |||
| <exclude>UnknownFieldSetLiteTest.java</exclude> | |||
Contributor
There was a problem hiding this comment.
not new, but why are we excluding the lite test here? Worth a look.
Author
There was a problem hiding this comment.
Hmm, good question. I'm not sure why.
acozzette
pushed a commit
to acozzette/protobuf
that referenced
this pull request
Jan 5, 2022
…s#9371) Credit should go to @elharo for most of these Java changes--I am just cherry-picking them from our internal codebase. The one thing I did change was to give the UTF-8 validation tests their own Bazel test target. This makes it possible to give the other tests a shorter timeout, which is important for UnknownFieldSetPerformanceTest in particular.
acozzette
pushed a commit
to acozzette/protobuf
that referenced
this pull request
Jan 5, 2022
…s#9371) Credit should go to @elharo for most of these Java changes--I am just cherry-picking them from our internal codebase. The one thing I did change was to give the UTF-8 validation tests their own Bazel test target. This makes it possible to give the other tests a shorter timeout, which is important for UnknownFieldSetPerformanceTest in particular.
acozzette
pushed a commit
to acozzette/protobuf
that referenced
this pull request
Jan 5, 2022
…s#9371) Credit should go to @elharo for most of these Java changes--I am just cherry-picking them from our internal codebase. The one thing I did change was to give the UTF-8 validation tests their own Bazel test target. This makes it possible to give the other tests a shorter timeout, which is important for UnknownFieldSetPerformanceTest in particular.
acozzette
pushed a commit
that referenced
this pull request
Jan 5, 2022
Credit should go to @elharo for most of these Java changes--I am just cherry-picking them from our internal codebase. The one thing I did change was to give the UTF-8 validation tests their own Bazel test target. This makes it possible to give the other tests a shorter timeout, which is important for UnknownFieldSetPerformanceTest in particular.
acozzette
pushed a commit
that referenced
this pull request
Jan 5, 2022
Credit should go to @elharo for most of these Java changes--I am just cherry-picking them from our internal codebase. The one thing I did change was to give the UTF-8 validation tests their own Bazel test target. This makes it possible to give the other tests a shorter timeout, which is important for UnknownFieldSetPerformanceTest in particular.
acozzette
pushed a commit
to acozzette/protobuf
that referenced
this pull request
Jan 5, 2022
…s#9371) Credit should go to @elharo for most of these Java changes--I am just cherry-picking them from our internal codebase. The one thing I did change was to give the UTF-8 validation tests their own Bazel test target. This makes it possible to give the other tests a shorter timeout, which is important for UnknownFieldSetPerformanceTest in particular.
acozzette
pushed a commit
to acozzette/protobuf
that referenced
this pull request
Jan 5, 2022
…s#9371) Credit should go to @elharo for most of these Java changes--I am just cherry-picking them from our internal codebase. The one thing I did change was to give the UTF-8 validation tests their own Bazel test target. This makes it possible to give the other tests a shorter timeout, which is important for UnknownFieldSetPerformanceTest in particular.
acozzette
pushed a commit
that referenced
this pull request
Jan 5, 2022
Credit should go to @elharo for most of these Java changes--I am just cherry-picking them from our internal codebase. The one thing I did change was to give the UTF-8 validation tests their own Bazel test target. This makes it possible to give the other tests a shorter timeout, which is important for UnknownFieldSetPerformanceTest in particular.
|
Is this fix going to be applied on 3.17.x branch ? |
Author
|
No, we're no longer maintaining the 3.17.x branch, so to get this fix you will have to upgrade to at least 3.18.2. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Credit should go to @elharo for most of these Java changes--I am just
cherry-picking them from our internal codebase. The one thing I did
change was to give the UTF-8 validation tests their own Bazel test
target. This makes it possible to give the other tests a shorter
timeout, which is important for UnknownFieldSetPerformanceTest in
particular.