-
Notifications
You must be signed in to change notification settings - Fork 25.7k
Speed up CompressedXContent.equals #79600
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
Changes from 8 commits
e92015c
0b60dbe
625d519
8e4809b
9ab5636
2ebb636
e535427
4b4ea40
edf8bf2
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -11,15 +11,17 @@ | |||||||||||||||||||
| import org.apache.lucene.util.TestUtil; | ||||||||||||||||||||
| import org.elasticsearch.common.bytes.BytesReference; | ||||||||||||||||||||
| import org.elasticsearch.common.io.stream.BytesStreamOutput; | ||||||||||||||||||||
| import org.elasticsearch.test.ESTestCase; | ||||||||||||||||||||
| import org.elasticsearch.xcontent.ToXContent; | ||||||||||||||||||||
| import org.elasticsearch.xcontent.ToXContentFragment; | ||||||||||||||||||||
| import org.elasticsearch.xcontent.ToXContentObject; | ||||||||||||||||||||
| import org.elasticsearch.xcontent.XContentFactory; | ||||||||||||||||||||
| import org.elasticsearch.xcontent.XContentType; | ||||||||||||||||||||
| import org.elasticsearch.test.ESTestCase; | ||||||||||||||||||||
| import org.junit.Assert; | ||||||||||||||||||||
|
|
||||||||||||||||||||
| import java.io.IOException; | ||||||||||||||||||||
| import java.io.OutputStream; | ||||||||||||||||||||
| import java.util.Arrays; | ||||||||||||||||||||
| import java.util.Random; | ||||||||||||||||||||
|
|
||||||||||||||||||||
| import static org.hamcrest.Matchers.equalTo; | ||||||||||||||||||||
|
|
@@ -106,4 +108,30 @@ public void testToXContentFragment() throws IOException { | |||||||||||||||||||
| CompressedXContent compressedXContent = new CompressedXContent(toXContentFragment, XContentType.JSON, ToXContent.EMPTY_PARAMS); | ||||||||||||||||||||
| assertEquals("{\"field\":\"value\"}", compressedXContent.string()); | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
|
||||||||||||||||||||
| public void testEquals() throws IOException { | ||||||||||||||||||||
| final String[] randomJSON = | ||||||||||||||||||||
| generateRandomStringArray(1000, randomIntBetween(1, 512), false, true); | ||||||||||||||||||||
| assertNotNull(randomJSON); | ||||||||||||||||||||
| final BytesReference jsonDirect = BytesReference.bytes( | ||||||||||||||||||||
| XContentFactory.jsonBuilder().startObject().stringListField("arr", Arrays.asList(randomJSON)).endObject()); | ||||||||||||||||||||
| final CompressedXContent one = new CompressedXContent(jsonDirect); | ||||||||||||||||||||
| final CompressedXContent sameAsOne = new CompressedXContent((builder, params) -> | ||||||||||||||||||||
| builder.stringListField("arr", Arrays.asList(randomJSON)), XContentType.JSON, ToXContent.EMPTY_PARAMS); | ||||||||||||||||||||
| assertEquals(one, sameAsOne); | ||||||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we assert that the compressed bytes are not equal in this case? Also we are not exercising
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah I see Alan already approved this. I think this is a blocker, the rest of my comments are less important.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added a test for this directly now :) I didn't want to hard code a collision (or brute force one to begin with :D) and finding one at runtime takes too long as well.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd quite like to have an |
||||||||||||||||||||
| } | ||||||||||||||||||||
|
|
||||||||||||||||||||
| public void testEqualsWhenUncompressed() throws IOException { | ||||||||||||||||||||
| final String[] randomJSON1 = | ||||||||||||||||||||
| generateRandomStringArray(randomIntBetween(1, 1000), randomIntBetween(1, 512), false, false); | ||||||||||||||||||||
| final String[] randomJSON2 = randomValueOtherThanMany( | ||||||||||||||||||||
| arr -> Arrays.equals(arr, randomJSON1), | ||||||||||||||||||||
| () -> generateRandomStringArray(randomIntBetween(1, 1000), randomIntBetween(1, 512), false, true) | ||||||||||||||||||||
| ); | ||||||||||||||||||||
| final CompressedXContent one = new CompressedXContent((builder, params) -> | ||||||||||||||||||||
| builder.stringListField("arr", Arrays.asList(randomJSON1)), XContentType.JSON, ToXContent.EMPTY_PARAMS); | ||||||||||||||||||||
| final CompressedXContent two = new CompressedXContent((builder, params) -> | ||||||||||||||||||||
| builder.stringListField("arr", Arrays.asList(randomJSON2)), XContentType.JSON, ToXContent.EMPTY_PARAMS); | ||||||||||||||||||||
| assertFalse(CompressedXContent.equalsWhenUncompressed(one.compressed(), two.compressed())); | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Here you go
Suggested change
|
||||||||||||||||||||
| } | ||||||||||||||||||||
Uh oh!
There was an error while loading. Please reload this page.