Add recipe to add bulk read method to InputStream implementations#6383
Add recipe to add bulk read method to InputStream implementations#6383
Conversation
3112869 to
29d5b13
Compare
29d5b13 to
275752f
Compare
rewrite-java/src/main/java/org/openrewrite/java/cleanup/AddInputStreamBulkReadMethod.java
Outdated
Show resolved
Hide resolved
rewrite-java/src/main/java/org/openrewrite/java/cleanup/AddInputStreamBulkReadMethod.java
Outdated
Show resolved
Hide resolved
rewrite-java/src/main/java/org/openrewrite/java/cleanup/AddInputStreamBulkReadMethod.java
Outdated
Show resolved
Hide resolved
rewrite-java/src/main/java/org/openrewrite/java/cleanup/AddInputStreamBulkReadMethod.java
Outdated
Show resolved
Hide resolved
rewrite-java/src/main/java/org/openrewrite/java/cleanup/AddInputStreamBulkReadMethod.java
Outdated
Show resolved
Hide resolved
275752f to
265ca75
Compare
Java's default InputStream.read(byte[], int, int) calls the single-byte read() in a loop, causing up to 350x slower performance for bulk reads. This recipe detects InputStream subclasses that: - Override read() but not read(byte[], int, int) - Delegate to another InputStream (including subclasses like ZipInputStream) For simple delegation patterns, it adds the missing bulk read method. For complex bodies (side effects, transformations), it adds a search marker for manual review. Handles both anonymous and named classes, ternary and if-statement null check styles, and skips FilterInputStream subclasses.
265ca75 to
cd815bb
Compare
rewrite-java/src/main/java/org/openrewrite/java/cleanup/AddInputStreamBulkReadMethod.java
Outdated
Show resolved
Hide resolved
rewrite-java/src/main/java/org/openrewrite/java/cleanup/AddInputStreamBulkReadMethod.java
Outdated
Show resolved
Hide resolved
rewrite-java/src/main/java/org/openrewrite/java/cleanup/AddInputStreamBulkReadMethod.java
Outdated
Show resolved
Hide resolved
rewrite-java/src/main/java/org/openrewrite/java/cleanup/AddInputStreamBulkReadMethod.java
Outdated
Show resolved
Hide resolved
rewrite-java/src/main/java/org/openrewrite/java/cleanup/AddInputStreamBulkReadMethod.java
Outdated
Show resolved
Hide resolved
rewrite-java/src/main/java/org/openrewrite/java/cleanup/AddInputStreamBulkReadMethod.java
Outdated
Show resolved
Hide resolved
rewrite-java/src/main/java/org/openrewrite/java/cleanup/AddInputStreamBulkReadMethod.java
Outdated
Show resolved
Hide resolved
rewrite-java/src/main/java/org/openrewrite/java/cleanup/AddInputStreamBulkReadMethod.java
Outdated
Show resolved
Hide resolved
rewrite-java/src/main/java/org/openrewrite/java/cleanup/AddInputStreamBulkReadMethod.java
Outdated
Show resolved
Hide resolved
rewrite-java/src/main/java/org/openrewrite/java/cleanup/AddInputStreamBulkReadMethod.java
Outdated
Show resolved
Hide resolved
rewrite-java/src/main/java/org/openrewrite/java/cleanup/AddInputStreamBulkReadMethod.java
Outdated
Show resolved
Hide resolved
rewrite-java/src/main/java/org/openrewrite/java/cleanup/AddInputStreamBulkReadMethod.java
Outdated
Show resolved
Hide resolved
rewrite-java/src/main/java/org/openrewrite/java/cleanup/AddInputStreamBulkReadMethod.java
Outdated
Show resolved
Hide resolved
rewrite-java/src/main/java/org/openrewrite/java/cleanup/AddInputStreamBulkReadMethod.java
Outdated
Show resolved
Hide resolved
- Extract duplicated visitor logic to processInputStreamClass() - Use @value for AnalysisResult instead of manual constructor - Import Set/HashSet instead of using FQN - Rename test nested classes (Transform, NoChange, MarkForReview) - Update copyright year to 2025
| * See the License for the specific language governing permissions and | ||
| * limitations under the License. | ||
| */ | ||
| package org.openrewrite.java.cleanup; |
There was a problem hiding this comment.
This package only contains a few visitors at this moment, as we've moved the majority of the recipes that were formerly here into rewrite-static-analysis.
For this recipe I'm doubting on where to place it, but I feel perhaps https://github.com/openrewrite/rewrite-migrate-java/blob/b566417e5ef76dfa6b4026cb2c08b9a5b335cbe7/src/main/java/org/openrewrite/java/migrate/io is more fitting than having it here.
If we keep it here, it would also be included with any other language module as those tend to use rewrite-java, which feels a bit off.
Adds a recipe to detect and fix InputStream subclasses that only override the single-byte read() method. Java's default bulk read implementation calls read() in a loop, which can cause up to 350x slower performance. For simple delegation patterns, the recipe adds the missing bulk read method. For complex bodies (side effects, transformations), it adds a search marker for manual review. Moved from openrewrite/rewrite#6383 per review feedback.
|
Moved to openrewrite/rewrite-migrate-java#961 per @timtebeek's review feedback. |
|
Closing in favor of openrewrite/rewrite-migrate-java#961 |
* Add bulk read method to InputStream implementations Adds a recipe to detect and fix InputStream subclasses that only override the single-byte read() method. Java's default bulk read implementation calls read() in a loop, which can cause up to 350x slower performance. For simple delegation patterns, the recipe adds the missing bulk read method. For complex bodies (side effects, transformations), it adds a search marker for manual review. Moved from openrewrite/rewrite#6383 per review feedback. * Apply the first automated suggestions * Add line to recipes.csv * Apply formatter and further best practices * Add placeholder for expanded precondition support upstream * Add precondition and inline conditionals logic --------- Co-authored-by: Tim te Beek <[email protected]>
Java's default
InputStream.read(byte[], int, int)calls the single-byteread()in a loop, causing up to 350x slower performance for bulk reads.This recipe detects InputStream subclasses that:
read()but notread(byte[], int, int)InputStreamFor simple delegation patterns, it adds the missing bulk read method. For complex bodies (side effects, transformations), it adds a search marker for manual review.
Handles both anonymous and named classes, ternary and if-statement null check styles, and skips FilterInputStream subclasses.
What's changed?
Adding a recipe to detect and update (simple cases of) InputStream implementations that are missing the bulk read method.
What's your motivation?
I found 3 such instances in OpenRewrite itself which caused huge performance gains when fixed.
Anything in particular you'd like reviewers to focus on?
Is this recipe useful in its current form, should it maybe only detect? or is only fixing simple cases good too?
Anyone you would like to review specifically?
@timtebeek
Have you considered any alternatives or workarounds?
Not creating this recipe
Any additional context
Checklist