Skip to content

Commit

Permalink
Make o.e.index.codec.ForUtil a pure utility class (elastic#107348)
Browse files Browse the repository at this point in the history 
We've run into heap dumps that had instances of this class consume tens
and in one case more than a hundred MB of heap.
It seems reasonable to use a thread-local for the `tmp` long array
and trade the cost of looking up the thread-local for the memory
savings and cycles saved for allocating and assigning instances.
  • Loading branch information
@original-brownbear
original-brownbear authored Apr 11, 2024
1 parent 07aa9cd commit 801b013
Show file tree
Hide file tree
Showing 7 changed files with 30 additions and 35 deletions.
15 changes: 10 additions & 5 deletions server/src/main/java/org/elasticsearch/index/codec/ForUtil.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,9 @@ public final class ForUtil {

public static final int BLOCK_SIZE = 128;
private static final int BLOCK_SIZE_LOG2 = 7;
private final long[] tmp = new long[BLOCK_SIZE / 2];
private static final ThreadLocal<long[]> scratch = ThreadLocal.withInitial(() -> new long[BLOCK_SIZE / 2]);

private ForUtil() {}

private static long expandMask32(long mask32) {
return mask32 | (mask32 << 32);
Expand Down Expand Up @@ -118,7 +120,7 @@ private static void collapse32(long[] arr) {
}

/** Encode 128 integers from {@code longs} into {@code out}. */
public void encode(long[] longs, int bitsPerValue, DataOutput out) throws IOException {
public static void encode(long[] longs, int bitsPerValue, DataOutput out) throws IOException {
final int nextPrimitive;
final int numLongs;
if (bitsPerValue <= 8) {
Expand All @@ -138,6 +140,7 @@ public void encode(long[] longs, int bitsPerValue, DataOutput out) throws IOExce
final int numLongsPerShift = bitsPerValue * 2;
int idx = 0;
int shift = nextPrimitive - bitsPerValue;
final long[] tmp = scratch.get();
for (int i = 0; i < numLongsPerShift; ++i) {
tmp[i] = longs[idx++] << shift;
}
Expand Down Expand Up @@ -191,7 +194,7 @@ public void encode(long[] longs, int bitsPerValue, DataOutput out) throws IOExce
}

/** Number of bytes required to encode 128 integers of {@code bitsPerValue} bits per value. */
public int numBytes(int bitsPerValue) {
public static int numBytes(int bitsPerValue) {
return bitsPerValue << (BLOCK_SIZE_LOG2 - 3);
}

Expand Down Expand Up @@ -299,7 +302,8 @@ private static void shiftLongs(long[] a, int count, long[] b, int bi, int shift,
private static final long MASK32_24 = MASKS32[24];

/** Decode 128 integers into {@code longs}. */
public void decode(int bitsPerValue, DataInput in, long[] longs) throws IOException {
public static void decode(int bitsPerValue, DataInput in, long[] longs) throws IOException {
final long[] tmp = scratch.get();
switch (bitsPerValue) {
case 1:
decode1(in, tmp, longs);
Expand Down Expand Up @@ -410,7 +414,8 @@ public void decode(int bitsPerValue, DataInput in, long[] longs) throws IOExcept
* [0..63], and values [64..127] are encoded in the low-order bits of {@code longs} [0..63]. This
* representation may allow subsequent operations to be performed on two values at a time.
*/
public void decodeTo32(int bitsPerValue, DataInput in, long[] longs) throws IOException {
public static void decodeTo32(int bitsPerValue, DataInput in, long[] longs) throws IOException {
final long[] tmp = scratch.get();
switch (bitsPerValue) {
case 1:
decode1(in, tmp, longs);
Expand Down
14 changes: 6 additions & 8 deletions server/src/main/java/org/elasticsearch/index/codec/postings/ES812PostingsReader.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,8 +61,6 @@ final class ES812PostingsReader extends PostingsReaderBase {
private final IndexInput posIn;
private final IndexInput payIn;

private final int version;

/** Sole constructor. */
ES812PostingsReader(SegmentReadState state) throws IOException {
boolean success = false;
Expand All @@ -78,7 +76,7 @@ final class ES812PostingsReader extends PostingsReaderBase {
String docName = IndexFileNames.segmentFileName(state.segmentInfo.name, state.segmentSuffix, ES812PostingsFormat.DOC_EXTENSION);
try {
docIn = state.directory.openInput(docName, state.context);
version = CodecUtil.checkIndexHeader(
int version = CodecUtil.checkIndexHeader(
docIn,
DOC_CODEC,
VERSION_START,
Expand Down Expand Up @@ -279,7 +277,7 @@ public ImpactsEnum impacts(FieldInfo fieldInfo, BlockTermState state, int flags)

final class BlockDocsEnum extends PostingsEnum {

final PForUtil pforUtil = new PForUtil(new ForUtil());
final PForUtil pforUtil = new PForUtil();

private final long[] docBuffer = new long[BLOCK_SIZE + 1];
private final long[] freqBuffer = new long[BLOCK_SIZE];
Expand Down Expand Up @@ -526,7 +524,7 @@ public long cost() {
// Also handles payloads + offsets
final class EverythingEnum extends PostingsEnum {

final PForUtil pforUtil = new PForUtil(new ForUtil());
final PForUtil pforUtil = new PForUtil();

private final long[] docBuffer = new long[BLOCK_SIZE + 1];
private final long[] freqBuffer = new long[BLOCK_SIZE + 1];
Expand Down Expand Up @@ -999,7 +997,7 @@ public long cost() {

final class BlockImpactsDocsEnum extends ImpactsEnum {

final PForUtil pforUtil = new PForUtil(new ForUtil());
final PForUtil pforUtil = new PForUtil();

private final long[] docBuffer = new long[BLOCK_SIZE + 1];
private final long[] freqBuffer = new long[BLOCK_SIZE];
Expand Down Expand Up @@ -1196,7 +1194,7 @@ public long cost() {

final class BlockImpactsPostingsEnum extends ImpactsEnum {

final PForUtil pforUtil = new PForUtil(new ForUtil());
final PForUtil pforUtil = new PForUtil();

private final long[] docBuffer = new long[BLOCK_SIZE];
private final long[] freqBuffer = new long[BLOCK_SIZE];
Expand Down Expand Up @@ -1476,7 +1474,7 @@ public long cost() {

final class BlockImpactsEverythingEnum extends ImpactsEnum {

final PForUtil pforUtil = new PForUtil(new ForUtil());
final PForUtil pforUtil = new PForUtil();

private final long[] docBuffer = new long[BLOCK_SIZE];
private final long[] freqBuffer = new long[BLOCK_SIZE];
Expand Down
3 changes: 1 addition & 2 deletions server/src/main/java/org/elasticsearch/index/codec/postings/ES812PostingsWriter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@
import org.apache.lucene.util.BitUtil;
import org.apache.lucene.util.BytesRef;
import org.elasticsearch.core.IOUtils;
import org.elasticsearch.index.codec.ForUtil;
import org.elasticsearch.index.codec.postings.ES812PostingsFormat.IntBlockTermState;

import java.io.IOException;
Expand Down Expand Up @@ -110,7 +109,7 @@ final class ES812PostingsWriter extends PushPostingsWriterBase {
boolean success = false;
try {
CodecUtil.writeIndexHeader(docOut, DOC_CODEC, VERSION_CURRENT, state.segmentInfo.getId(), state.segmentSuffix);
pforUtil = new PForUtil(new ForUtil());
pforUtil = new PForUtil();
if (state.fieldInfos.hasProx()) {
posDeltaBuffer = new long[BLOCK_SIZE];
String posFileName = IndexFileNames.segmentFileName(
Expand Down
14 changes: 6 additions & 8 deletions server/src/main/java/org/elasticsearch/index/codec/postings/PForUtil.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,14 +52,12 @@ static boolean allEqual(long[] l) {
return true;
}

private final ForUtil forUtil;
// buffer for reading exception data; each exception uses two bytes (pos + high-order bits of the
// exception)
private final byte[] exceptionBuff = new byte[MAX_EXCEPTIONS * 2];

PForUtil(ForUtil forUtil) {
PForUtil() {
assert ForUtil.BLOCK_SIZE <= 256 : "blocksize must fit in one byte. got " + ForUtil.BLOCK_SIZE;
this.forUtil = forUtil;
}

/** Encode 128 integers from {@code longs} into {@code out}. */
Expand Down Expand Up @@ -114,7 +112,7 @@ void encode(long[] longs, DataOutput out) throws IOException {
} else {
final int token = (numExceptions << 5) | patchedBitsRequired;
out.writeByte((byte) token);
forUtil.encode(longs, patchedBitsRequired, out);
ForUtil.encode(longs, patchedBitsRequired, out);
}
out.writeBytes(exceptions, exceptions.length);
}
Expand All @@ -127,7 +125,7 @@ void decode(DataInput in, long[] longs) throws IOException {
if (bitsPerValue == 0) {
Arrays.fill(longs, 0, ForUtil.BLOCK_SIZE, in.readVLong());
} else {
forUtil.decode(bitsPerValue, in, longs);
ForUtil.decode(bitsPerValue, in, longs);
}
for (int i = 0; i < numExceptions; ++i) {
longs[Byte.toUnsignedInt(in.readByte())] |= Byte.toUnsignedLong(in.readByte()) << bitsPerValue;
Expand All @@ -153,15 +151,15 @@ void decodeAndPrefixSum(DataInput in, long base, long[] longs) throws IOExceptio
}
} else {
// decode the deltas then apply the prefix sum logic
forUtil.decodeTo32(bitsPerValue, in, longs);
ForUtil.decodeTo32(bitsPerValue, in, longs);
prefixSum32(longs, base);
}
} else {
// pack two values per long so we can apply prefixes two-at-a-time
if (bitsPerValue == 0) {
fillSameValue32(longs, in.readVLong());
} else {
forUtil.decodeTo32(bitsPerValue, in, longs);
ForUtil.decodeTo32(bitsPerValue, in, longs);
}
applyExceptions32(bitsPerValue, numExceptions, in, longs);
prefixSum32(longs, base);
Expand All @@ -177,7 +175,7 @@ void skip(DataInput in) throws IOException {
in.readVLong();
in.skipBytes((numExceptions << 1));
} else {
in.skipBytes(forUtil.numBytes(bitsPerValue) + (numExceptions << 1));
in.skipBytes(ForUtil.numBytes(bitsPerValue) + (numExceptions << 1));
}
}

Expand Down
5 changes: 2 additions & 3 deletions server/src/main/java/org/elasticsearch/index/codec/tsdb/DocValuesForUtil.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ public class DocValuesForUtil {
private static final int BITS_IN_FIVE_BYTES = 5 * Byte.SIZE;
private static final int BITS_IN_SIX_BYTES = 6 * Byte.SIZE;
private static final int BITS_IN_SEVEN_BYTES = 7 * Byte.SIZE;
private final ForUtil forUtil = new ForUtil();
private final int blockSize;
private final byte[] encoded;

Expand Down Expand Up @@ -50,7 +49,7 @@ public static int roundBits(int bitsPerValue) {

public void encode(long[] in, int bitsPerValue, final DataOutput out) throws IOException {
if (bitsPerValue <= 24) { // these bpvs are handled efficiently by ForUtil
forUtil.encode(in, bitsPerValue, out);
ForUtil.encode(in, bitsPerValue, out);
} else if (bitsPerValue <= 32) {
collapse32(in);
for (int i = 0; i < blockSize / 2; ++i) {
Expand All @@ -76,7 +75,7 @@ private void encodeFiveSixOrSevenBytesPerValue(long[] in, int bitsPerValue, fina

public void decode(int bitsPerValue, final DataInput in, long[] out) throws IOException {
if (bitsPerValue <= 24) {
forUtil.decode(bitsPerValue, in, out);
ForUtil.decode(bitsPerValue, in, out);
} else if (bitsPerValue <= 32) {
in.readLongs(out, 0, blockSize / 2);
expand32(out);
Expand Down
8 changes: 3 additions & 5 deletions server/src/test/java/org/elasticsearch/index/codec/ForUtilTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,6 @@ public void testEncodeDecode() throws IOException {
{
// encode
IndexOutput out = d.createOutput("test.bin", IOContext.DEFAULT);
final ForUtil forUtil = new ForUtil();

for (int i = 0; i < iterations; ++i) {
long[] source = new long[ForUtil.BLOCK_SIZE];
Expand All @@ -64,7 +63,7 @@ public void testEncodeDecode() throws IOException {
}
final int bpv = PackedInts.bitsRequired(or);
out.writeByte((byte) bpv);
forUtil.encode(source, bpv, out);
ForUtil.encode(source, bpv, out);
}
endPointer = out.getFilePointer();
out.close();
Expand All @@ -73,12 +72,11 @@ public void testEncodeDecode() throws IOException {
{
// decode
IndexInput in = d.openInput("test.bin", IOContext.READONCE);
final ForUtil forUtil = new ForUtil();
for (int i = 0; i < iterations; ++i) {
final int bitsPerValue = in.readByte();
final long currentFilePointer = in.getFilePointer();
final long[] restored = new long[ForUtil.BLOCK_SIZE];
forUtil.decode(bitsPerValue, in, restored);
ForUtil.decode(bitsPerValue, in, restored);
int[] ints = new int[ForUtil.BLOCK_SIZE];
for (int j = 0; j < ForUtil.BLOCK_SIZE; ++j) {
ints[j] = Math.toIntExact(restored[j]);
Expand All @@ -88,7 +86,7 @@ public void testEncodeDecode() throws IOException {
ArrayUtil.copyOfSubArray(values, i * ForUtil.BLOCK_SIZE, (i + 1) * ForUtil.BLOCK_SIZE),
ints
);
assertEquals(forUtil.numBytes(bitsPerValue), in.getFilePointer() - currentFilePointer);
assertEquals(ForUtil.numBytes(bitsPerValue), in.getFilePointer() - currentFilePointer);
}
assertEquals(endPointer, in.getFilePointer());
in.close();
Expand Down
6 changes: 2 additions & 4 deletions server/src/test/java/org/elasticsearch/index/codec/tsdb/DocValuesForUtilTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,6 @@
import java.util.Random;

public class DocValuesForUtilTests extends LuceneTestCase {
private final ES87TSDBDocValuesEncoder encoder = new ES87TSDBDocValuesEncoder();
private static final ForUtil forUtil = new ForUtil();

public void testEncodeDecode() throws IOException {
final int iterations = RandomNumbers.randomIntBetween(random(), 50, 1000);
Expand Down Expand Up @@ -105,14 +103,14 @@ public void testEncodeDecodeBitsPerValue() throws IOException {
// Encode
DataOutput dataOutput = new ByteArrayDataOutput(dataOutputBuffer);
long[] encodeBuffer = Arrays.copyOf(values, values.length);
forUtil.encode(encodeBuffer, bitsPerValue, dataOutput);
ForUtil.encode(encodeBuffer, bitsPerValue, dataOutput);

// Prepare for decoding
DataInput dataInput = new ByteArrayDataInput(dataInputBuffer);
System.arraycopy(dataOutputBuffer, 0, dataInputBuffer, 0, dataOutputBuffer.length);

// Decode
forUtil.decode(bitsPerValue, dataInput, decodeBuffer);
ForUtil.decode(bitsPerValue, dataInput, decodeBuffer);

assertArrayEquals(decodeBuffer, values);
}
Expand Down

0 comments on commit 801b013

Please sign in to comment.