tommyettinger opened a new pull request, #1573: URL: https://github.com/apache/incubator-fury/pull/1573
<!-- **Thanks for contributing to Fury.** **If this is your first time opening a PR on fury, you can refer to [CONTRIBUTING.md](https://github.com/apache/incubator-fury/blob/main/CONTRIBUTING.md).** Contribution Checklist - The **Apache Fury (incubating)** community has restrictions on the naming of pr titles. You can also find instructions in [CONTRIBUTING.md](https://github.com/apache/incubator-fury/blob/main/CONTRIBUTING.md). - (Done.) - Fury has a strong focus on performance. If the PR you submit will have an impact on performance, please benchmark it first and provide the benchmark result here. - (This should not affect performance at all.) --> ## What does this PR do? Without this change, if `ObjectArray.clearObjectArray()` is called with start > 0 and size < 128 (which is `COPY_THRESHOLD`), then an IllegalArgumentException is thrown. This happens because `Arrays.fill()` takes a from index and a to index, in contrast with `clearObjectArray()`, which takes a start index and a size. The exception looks like this: ``` Exception in thread "main" java.lang.IllegalArgumentException: fromIndex(11) > toIndex(5) at java.base/java.util.Arrays.rangeCheck(Arrays.java:718) at java.base/java.util.Arrays.fill(Arrays.java:3453) ``` This wasn't thrown in Fury code, because Fury itself never calls clearObjectArray() with any start value other than 0. The method is part of a public API, though, so user code can call it. I added an extra check to ObjectArrayTest that calls clearObjectArray() with 1 as a start value, instead of only testing with a start value of 0. I can run the tests one at a time and the only test I touched passes, but (maybe because of the commit I checked out) running the command in CONTRIBUTING.md (`mvn -T10 clean test`) fails on `org.apache.fury.serializer.MetaSharedCompatibleTest`. I'm not sure if there's anything I can do about that; the issue may already be fixed. Spotless and Checkstyle have both been run and found no issues. ## Related issues None. ## Does this PR introduce any user-facing change? No. - [ ] Does this PR introduce any public API change? - [ ] Does this PR introduce any binary protocol compatibility change? ## Benchmark I don't see any way this could have a significant effect on performance. It only adds one addition performed once during some calls to an O(n) function, and clearing an array can't be done multiple times in sequence with any difference from doing it once. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
