Skip to content

Commit 27dcbcd

Browse files
committed
[SPARK-52880][CORE] Improve toString by JEP-280 instead of ToStringBuilder
### What changes were proposed in this pull request? This PR aims to improve `toString` by `JEP-280` instead of `ToStringBuilder`. In addition, `Scalastyle` and `Checkstyle` rules are added to prevent a future regression. ### Why are the changes needed? Since Java 9, `String Concatenation` has been handled better by default. | ID | DESCRIPTION | | - | - | | JEP-280 | [Indify String Concatenation](https://openjdk.org/jeps/280) | For example, this PR improves `OpenBlocks` like the following. Both Java source code and byte code are simplified a lot by utilizing JEP-280 properly. **CODE CHANGE** ```java - return new ToStringBuilder(this, ToStringStyle.SHORT_PREFIX_STYLE) - .append("appId", appId) - .append("execId", execId) - .append("blockIds", Arrays.toString(blockIds)) - .toString(); + return "OpenBlocks[appId=" + appId + ",execId=" + execId + ",blockIds=" + + Arrays.toString(blockIds) + "]"; ``` **BEFORE** ``` public java.lang.String toString(); Code: 0: new #39 // class org/apache/commons/lang3/builder/ToStringBuilder 3: dup 4: aload_0 5: getstatic #41 // Field org/apache/commons/lang3/builder/ToStringStyle.SHORT_PREFIX_STYLE:Lorg/apache/commons/lang3/builder/ToStringStyle; 8: invokespecial #47 // Method org/apache/commons/lang3/builder/ToStringBuilder."<init>":(Ljava/lang/Object;Lorg/apache/commons/lang3/builder/ToStringStyle;)V 11: ldc #50 // String appId 13: aload_0 14: getfield #7 // Field appId:Ljava/lang/String; 17: invokevirtual #51 // Method org/apache/commons/lang3/builder/ToStringBuilder.append:(Ljava/lang/String;Ljava/lang/Object;)Lorg/apache/commons/lang3/builder/ToStringBuilder; 20: ldc #55 // String execId 22: aload_0 23: getfield #13 // Field execId:Ljava/lang/String; 26: invokevirtual #51 // Method org/apache/commons/lang3/builder/ToStringBuilder.append:(Ljava/lang/String;Ljava/lang/Object;)Lorg/apache/commons/lang3/builder/ToStringBuilder; 29: ldc #56 // String blockIds 31: aload_0 32: getfield #16 // Field blockIds:[Ljava/lang/String; 35: invokestatic #57 // Method java/util/Arrays.toString:([Ljava/lang/Object;)Ljava/lang/String; 38: invokevirtual #51 // Method org/apache/commons/lang3/builder/ToStringBuilder.append:(Ljava/lang/String;Ljava/lang/Object;)Lorg/apache/commons/lang3/builder/ToStringBuilder; 41: invokevirtual #61 // Method org/apache/commons/lang3/builder/ToStringBuilder.toString:()Ljava/lang/String; 44: areturn ``` **AFTER** ``` public java.lang.String toString(); Code: 0: aload_0 1: getfield #7 // Field appId:Ljava/lang/String; 4: aload_0 5: getfield #13 // Field execId:Ljava/lang/String; 8: aload_0 9: getfield #16 // Field blockIds:[Ljava/lang/String; 12: invokestatic #39 // Method java/util/Arrays.toString:([Ljava/lang/Object;)Ljava/lang/String; 15: invokedynamic #43, 0 // InvokeDynamic #0:makeConcatWithConstants:(Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;)Ljava/lang/String; 20: areturn ``` ### Does this PR introduce _any_ user-facing change? No. This is an `toString` implementation improvement. ### How was this patch tested? Pass the CIs. ### Was this patch authored or co-authored using generative AI tooling? No. Closes #51572 from dongjoon-hyun/SPARK-52880. Authored-by: Dongjoon Hyun <[email protected]> Signed-off-by: Dongjoon Hyun <[email protected]>
1 parent 0177265 commit 27dcbcd

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

43 files changed

+80
-269
lines changed

common/network-common/src/main/java/org/apache/spark/network/buffer/FileSegmentManagedBuffer.java

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -29,8 +29,6 @@
2929
import com.google.common.io.ByteStreams;
3030
import io.netty.channel.DefaultFileRegion;
3131
import io.netty.handler.stream.ChunkedStream;
32-
import org.apache.commons.lang3.builder.ToStringBuilder;
33-
import org.apache.commons.lang3.builder.ToStringStyle;
3432

3533
import org.apache.spark.network.util.JavaUtils;
3634
import org.apache.spark.network.util.LimitedInputStream;
@@ -152,10 +150,7 @@ public Object convertToNettyForSsl() throws IOException {
152150

153151
@Override
154152
public String toString() {
155-
return new ToStringBuilder(this, ToStringStyle.SHORT_PREFIX_STYLE)
156-
.append("file", file)
157-
.append("offset", offset)
158-
.append("length", length)
159-
.toString();
153+
return "FileSegmentManagedBuffer[file=" + file + ",offset=" + offset +
154+
",length=" + length + "]";
160155
}
161156
}

common/network-common/src/main/java/org/apache/spark/network/buffer/NettyManagedBuffer.java

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,6 @@
2323

2424
import io.netty.buffer.ByteBuf;
2525
import io.netty.buffer.ByteBufInputStream;
26-
import org.apache.commons.lang3.builder.ToStringBuilder;
27-
import org.apache.commons.lang3.builder.ToStringStyle;
2826

2927
/**
3028
* A {@link ManagedBuffer} backed by a Netty {@link ByteBuf}.
@@ -75,8 +73,6 @@ public Object convertToNettyForSsl() throws IOException {
7573

7674
@Override
7775
public String toString() {
78-
return new ToStringBuilder(this, ToStringStyle.SHORT_PREFIX_STYLE)
79-
.append("buf", buf)
80-
.toString();
76+
return "NettyManagedBuffer[buf=" + buf + "]";
8177
}
8278
}

common/network-common/src/main/java/org/apache/spark/network/buffer/NioManagedBuffer.java

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,6 @@
2323

2424
import io.netty.buffer.ByteBufInputStream;
2525
import io.netty.buffer.Unpooled;
26-
import org.apache.commons.lang3.builder.ToStringBuilder;
27-
import org.apache.commons.lang3.builder.ToStringStyle;
2826

2927
/**
3028
* A {@link ManagedBuffer} backed by {@link ByteBuffer}.
@@ -73,9 +71,7 @@ public Object convertToNettyForSsl() throws IOException {
7371

7472
@Override
7573
public String toString() {
76-
return new ToStringBuilder(this, ToStringStyle.SHORT_PREFIX_STYLE)
77-
.append("buf", buf)
78-
.toString();
74+
return "NioManagedBuffer[buf=" + buf + "]";
7975
}
8076
}
8177

common/network-common/src/main/java/org/apache/spark/network/client/TransportClient.java

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -33,8 +33,6 @@
3333
import io.netty.channel.Channel;
3434
import io.netty.util.concurrent.Future;
3535
import io.netty.util.concurrent.GenericFutureListener;
36-
import org.apache.commons.lang3.builder.ToStringBuilder;
37-
import org.apache.commons.lang3.builder.ToStringStyle;
3836

3937
import org.apache.spark.internal.SparkLogger;
4038
import org.apache.spark.internal.SparkLoggerFactory;
@@ -338,11 +336,8 @@ public void close() {
338336

339337
@Override
340338
public String toString() {
341-
return new ToStringBuilder(this, ToStringStyle.SHORT_PREFIX_STYLE)
342-
.append("remoteAddress", channel.remoteAddress())
343-
.append("clientId", clientId)
344-
.append("isActive", isActive())
345-
.toString();
339+
return "TransportClient[remoteAddress=" + channel.remoteAddress() + "clientId=" + clientId +
340+
",isActive=" + isActive() + "]";
346341
}
347342

348343
private static long requestId() {

common/network-common/src/main/java/org/apache/spark/network/protocol/ChunkFetchFailure.java

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,6 @@
2020
import java.util.Objects;
2121

2222
import io.netty.buffer.ByteBuf;
23-
import org.apache.commons.lang3.builder.ToStringBuilder;
24-
import org.apache.commons.lang3.builder.ToStringStyle;
2523

2624
/**
2725
* Response to {@link ChunkFetchRequest} when there is an error fetching the chunk.
@@ -70,9 +68,6 @@ public boolean equals(Object other) {
7068

7169
@Override
7270
public String toString() {
73-
return new ToStringBuilder(this, ToStringStyle.SHORT_PREFIX_STYLE)
74-
.append("streamChunkId", streamChunkId)
75-
.append("errorString", errorString)
76-
.toString();
71+
return "ChunkFetchFailure[streamChunkId=" + streamChunkId + ",errorString=" + errorString + "]";
7772
}
7873
}

common/network-common/src/main/java/org/apache/spark/network/protocol/ChunkFetchRequest.java

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,6 @@
1818
package org.apache.spark.network.protocol;
1919

2020
import io.netty.buffer.ByteBuf;
21-
import org.apache.commons.lang3.builder.ToStringBuilder;
22-
import org.apache.commons.lang3.builder.ToStringStyle;
2321

2422
/**
2523
* Request to fetch a sequence of a single chunk of a stream. This will correspond to a single
@@ -64,8 +62,6 @@ public boolean equals(Object other) {
6462

6563
@Override
6664
public String toString() {
67-
return new ToStringBuilder(this, ToStringStyle.SHORT_PREFIX_STYLE)
68-
.append("streamChunkId", streamChunkId)
69-
.toString();
65+
return "ChunkFetchRequest[streamChunkId=" + streamChunkId + "]";
7066
}
7167
}

common/network-common/src/main/java/org/apache/spark/network/protocol/ChunkFetchSuccess.java

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,6 @@
2020
import java.util.Objects;
2121

2222
import io.netty.buffer.ByteBuf;
23-
import org.apache.commons.lang3.builder.ToStringBuilder;
24-
import org.apache.commons.lang3.builder.ToStringStyle;
2523

2624
import org.apache.spark.network.buffer.ManagedBuffer;
2725
import org.apache.spark.network.buffer.NettyManagedBuffer;
@@ -83,9 +81,6 @@ public boolean equals(Object other) {
8381

8482
@Override
8583
public String toString() {
86-
return new ToStringBuilder(this, ToStringStyle.SHORT_PREFIX_STYLE)
87-
.append("streamChunkId", streamChunkId)
88-
.append("body", body())
89-
.toString();
84+
return "ChunkFetchSuccess[streamChunkId=" + streamChunkId + ",body=" + body() + "]";
9085
}
9186
}

common/network-common/src/main/java/org/apache/spark/network/protocol/MergedBlockMetaRequest.java

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,6 @@
1919

2020
import com.google.common.base.Objects;
2121
import io.netty.buffer.ByteBuf;
22-
import org.apache.commons.lang3.builder.ToStringBuilder;
23-
import org.apache.commons.lang3.builder.ToStringStyle;
2422

2523
/**
2624
* Request to find the meta information for the specified merged block. The meta information
@@ -94,12 +92,7 @@ public boolean equals(Object other) {
9492

9593
@Override
9694
public String toString() {
97-
return new ToStringBuilder(this, ToStringStyle.SHORT_PREFIX_STYLE)
98-
.append("requestId", requestId)
99-
.append("appId", appId)
100-
.append("shuffleId", shuffleId)
101-
.append("shuffleMergeId", shuffleMergeId)
102-
.append("reduceId", reduceId)
103-
.toString();
95+
return "MergedBlockMetaRequest[requestId=" + requestId + ",appId=" + appId + ",shuffleId=" +
96+
shuffleId + ",shuffleMergeId=" + shuffleMergeId + ",reduceId=" + reduceId + "]";
10497
}
10598
}

common/network-common/src/main/java/org/apache/spark/network/protocol/MergedBlockMetaSuccess.java

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,6 @@
1919

2020
import com.google.common.base.Objects;
2121
import io.netty.buffer.ByteBuf;
22-
import org.apache.commons.lang3.builder.ToStringBuilder;
23-
import org.apache.commons.lang3.builder.ToStringStyle;
2422

2523
import org.apache.spark.network.buffer.ManagedBuffer;
2624
import org.apache.spark.network.buffer.NettyManagedBuffer;
@@ -56,8 +54,7 @@ public int hashCode() {
5654

5755
@Override
5856
public String toString() {
59-
return new ToStringBuilder(this, ToStringStyle.SHORT_PREFIX_STYLE)
60-
.append("requestId", requestId).append("numChunks", numChunks).toString();
57+
return "MergedBlockMetaSuccess[requestId=" + requestId + ",numChunks=" + numChunks + "]";
6158
}
6259

6360
@Override

common/network-common/src/main/java/org/apache/spark/network/protocol/OneWayMessage.java

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,6 @@
2020
import java.util.Objects;
2121

2222
import io.netty.buffer.ByteBuf;
23-
import org.apache.commons.lang3.builder.ToStringBuilder;
24-
import org.apache.commons.lang3.builder.ToStringStyle;
2523

2624
import org.apache.spark.network.buffer.ManagedBuffer;
2725
import org.apache.spark.network.buffer.NettyManagedBuffer;
@@ -74,8 +72,6 @@ public boolean equals(Object other) {
7472

7573
@Override
7674
public String toString() {
77-
return new ToStringBuilder(this, ToStringStyle.SHORT_PREFIX_STYLE)
78-
.append("body", body())
79-
.toString();
75+
return "OneWayMessage[body=" + body() + "]";
8076
}
8177
}

0 commit comments

Comments
 (0)