Skip to content

Commit ac38bdc

Browse files
tejasapatilsrowen
authored andcommitted
[SPARK-15601][CORE] CircularBuffer's toString() to print only the contents written if buffer isn't full
## What changes were proposed in this pull request? 1. The class allocated 4x space than needed as it was using `Int` to store the `Byte` values 2. If CircularBuffer isn't full, currently toString() will print some garbage chars along with the content written as is tries to print the entire array allocated for the buffer. The fix is to keep track of buffer getting full and don't print the tail of the buffer if it isn't full (suggestion by sameeragarwal over apache#12194 (comment)) 3. Simplified `toString()` ## How was this patch tested? Added new test case Author: Tejas Patil <tejasp@fb.com> Closes apache#13351 from tejasapatil/circular_buffer.
1 parent 04f925e commit ac38bdc

2 files changed

Lines changed: 43 additions & 25 deletions

File tree

core/src/main/scala/org/apache/spark/util/Utils.scala

Lines changed: 13 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -2344,29 +2344,24 @@ private[spark] class RedirectThread(
23442344
* the toString method.
23452345
*/
23462346
private[spark] class CircularBuffer(sizeInBytes: Int = 10240) extends java.io.OutputStream {
2347-
var pos: Int = 0
2348-
var buffer = new Array[Int](sizeInBytes)
2347+
private var pos: Int = 0
2348+
private var isBufferFull = false
2349+
private val buffer = new Array[Byte](sizeInBytes)
23492350

2350-
def write(i: Int): Unit = {
2351-
buffer(pos) = i
2351+
def write(input: Int): Unit = {
2352+
buffer(pos) = input.toByte
23522353
pos = (pos + 1) % buffer.length
2354+
isBufferFull = isBufferFull || (pos == 0)
23532355
}
23542356

23552357
override def toString: String = {
2356-
val (end, start) = buffer.splitAt(pos)
2357-
val input = new java.io.InputStream {
2358-
val iterator = (start ++ end).iterator
2359-
2360-
def read(): Int = if (iterator.hasNext) iterator.next() else -1
2361-
}
2362-
val reader = new BufferedReader(new InputStreamReader(input, StandardCharsets.UTF_8))
2363-
val stringBuilder = new StringBuilder
2364-
var line = reader.readLine()
2365-
while (line != null) {
2366-
stringBuilder.append(line)
2367-
stringBuilder.append("\n")
2368-
line = reader.readLine()
2358+
if (!isBufferFull) {
2359+
return new String(buffer, 0, pos, StandardCharsets.UTF_8)
23692360
}
2370-
stringBuilder.toString()
2361+
2362+
val nonCircularBuffer = new Array[Byte](sizeInBytes)
2363+
System.arraycopy(buffer, pos, nonCircularBuffer, 0, buffer.length - pos)
2364+
System.arraycopy(buffer, 0, nonCircularBuffer, buffer.length - pos, pos)
2365+
new String(nonCircularBuffer, StandardCharsets.UTF_8)
23712366
}
23722367
}

core/src/test/scala/org/apache/spark/util/UtilsSuite.scala

Lines changed: 30 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@
1717

1818
package org.apache.spark.util
1919

20-
import java.io.{ByteArrayInputStream, ByteArrayOutputStream, File, FileOutputStream}
20+
import java.io.{ByteArrayInputStream, ByteArrayOutputStream, File, FileOutputStream, PrintStream}
2121
import java.lang.{Double => JDouble, Float => JFloat}
2222
import java.net.{BindException, ServerSocket, URI}
2323
import java.nio.{ByteBuffer, ByteOrder}
@@ -681,14 +681,37 @@ class UtilsSuite extends SparkFunSuite with ResetSystemProperties with Logging {
681681
assert(!Utils.isInDirectory(nullFile, childFile3))
682682
}
683683

684-
test("circular buffer") {
684+
test("circular buffer: if nothing was written to the buffer, display nothing") {
685+
val buffer = new CircularBuffer(4)
686+
assert(buffer.toString === "")
687+
}
688+
689+
test("circular buffer: if the buffer isn't full, print only the contents written") {
690+
val buffer = new CircularBuffer(10)
691+
val stream = new PrintStream(buffer, true, "UTF-8")
692+
stream.print("test")
693+
assert(buffer.toString === "test")
694+
}
695+
696+
test("circular buffer: data written == size of the buffer") {
697+
val buffer = new CircularBuffer(4)
698+
val stream = new PrintStream(buffer, true, "UTF-8")
699+
700+
// fill the buffer to its exact size so that it just hits overflow
701+
stream.print("test")
702+
assert(buffer.toString === "test")
703+
704+
// add more data to the buffer
705+
stream.print("12")
706+
assert(buffer.toString === "st12")
707+
}
708+
709+
test("circular buffer: multiple overflow") {
685710
val buffer = new CircularBuffer(25)
686-
val stream = new java.io.PrintStream(buffer, true, "UTF-8")
711+
val stream = new PrintStream(buffer, true, "UTF-8")
687712

688-
// scalastyle:off println
689-
stream.println("test circular test circular test circular test circular test circular")
690-
// scalastyle:on println
691-
assert(buffer.toString === "t circular test circular\n")
713+
stream.print("test circular test circular test circular test circular test circular")
714+
assert(buffer.toString === "st circular test circular")
692715
}
693716

694717
test("nanSafeCompareDoubles") {

0 commit comments

Comments
 (0)