[PARQUET-2202] Redundant String allocation on the hot path in CapacityByteArrayOutputStream.setByte

• Type: Bug
• Status: Resolved
• Priority: Major
• Resolution: Fixed
• Affects Version/s: 1.12.3
• Fix Version/s: 1.13.0
• Component/s: parquet-mr
• Labels: performance
• Assignee: Unassigned
• Reporter: Andrei Pangin
• Created: 09/Oct/22 22:57
• Updated: 12/Apr/23 01:31
• Resolved: 26/Mar/23 05:42

Description

Profiling of a Spark application revealed a performance issue in production:

CapacityByteArrayOutputStream.setByte consumed 2.2% of total CPU time and made up 4.6% of total allocations. However, in normal case, this method should allocate nothing at all.

Here is an excerpt from async-profiler report.

CPU profile:

Allocation profile:

The reason is a checkArgument() call with an unconditionally constructed dynamic String:

https://github.com/apache/parquet-mr/blob/62b774cd0f0c60cfbe540bbfa60bee15929af5d4/parquet-common/src/main/java/org/apache/parquet/bytes/CapacityByteArrayOutputStream.java#L303

The suggested fix is to move String construction under the condition:

if (index >= bytesUsed) {
  throw new IllegalArgumentException("Index: " + index +
      " is >= the current size of: " + bytesUsed);
}