Skip to content

Commit 785a91c

Browse files
Optimize some Spots around Closing Resources (#60049)
The single element `close` calls go through a very inefficient path that includes creating a one element list. `releaseOnce` is only with a single non-null input in production in two spots so no need for varargs and any complexity here. `ReleasableBytesStreamOutput` does not require any `releaseOnce` wrapping because we already have that kind of logic implemented in `org.elasticsearch.common.util.AbstractArray` (which we were wrapping here) already.
1 parent 453298f commit 785a91c

File tree

4 files changed

+31
-44
lines changed

4 files changed

+31
-44
lines changed

libs/core/src/main/java/org/elasticsearch/core/internal/io/IOUtils.java

+23-10
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,8 @@
1919

2020
package org.elasticsearch.core.internal.io;
2121

22+
import org.elasticsearch.common.Nullable;
23+
2224
import java.io.Closeable;
2325
import java.io.IOException;
2426
import java.nio.channels.FileChannel;
@@ -64,6 +66,15 @@ public static void close(final Closeable... objects) throws IOException {
6466
close(null, Arrays.asList(objects));
6567
}
6668

69+
/**
70+
* @see #close(Closeable...)
71+
*/
72+
public static void close(@Nullable Closeable closeable) throws IOException {
73+
if (closeable != null) {
74+
closeable.close();
75+
}
76+
}
77+
6778
/**
6879
* Closes all given {@link Closeable}s. Some of the {@linkplain Closeable}s may be null; they are
6980
* ignored. After everything is closed, the method adds any exceptions as suppressed to the
@@ -102,9 +113,7 @@ public static void close(final Exception ex, final Iterable<? extends Closeable>
102113
Exception firstException = ex;
103114
for (final Closeable object : objects) {
104115
try {
105-
if (object != null) {
106-
object.close();
107-
}
116+
close(object);
108117
} catch (final IOException | RuntimeException e) {
109118
if (firstException == null) {
110119
firstException = e;
@@ -142,14 +151,18 @@ public static void closeWhileHandlingException(final Closeable... objects) {
142151
*/
143152
public static void closeWhileHandlingException(final Iterable<? extends Closeable> objects) {
144153
for (final Closeable object : objects) {
145-
// noinspection EmptyCatchBlock
146-
try {
147-
if (object != null) {
148-
object.close();
149-
}
150-
} catch (final IOException | RuntimeException e) {
154+
closeWhileHandlingException(object);
155+
}
156+
}
151157

152-
}
158+
/**
159+
* @see #closeWhileHandlingException(Closeable...)
160+
*/
161+
public static void closeWhileHandlingException(final Closeable closeable) {
162+
// noinspection EmptyCatchBlock
163+
try {
164+
close(closeable);
165+
} catch (final IOException | RuntimeException e) {
153166
}
154167
}
155168

server/src/main/java/org/elasticsearch/common/io/stream/ReleasableBytesStreamOutput.java

+2-28
Original file line numberDiff line numberDiff line change
@@ -21,9 +21,7 @@
2121

2222
import org.elasticsearch.common.bytes.ReleasableBytesReference;
2323
import org.elasticsearch.common.lease.Releasable;
24-
import org.elasticsearch.common.lease.Releasables;
2524
import org.elasticsearch.common.util.BigArrays;
26-
import org.elasticsearch.common.util.ByteArray;
2725
import org.elasticsearch.common.util.PageCacheRecycler;
2826

2927
/**
@@ -35,42 +33,18 @@
3533
* stream should only be closed after the bytes have been output or copied
3634
* elsewhere.
3735
*/
38-
public class ReleasableBytesStreamOutput extends BytesStreamOutput
39-
implements Releasable {
40-
41-
private Releasable releasable;
36+
public class ReleasableBytesStreamOutput extends BytesStreamOutput implements Releasable {
4237

4338
public ReleasableBytesStreamOutput(BigArrays bigarrays) {
4439
this(PageCacheRecycler.PAGE_SIZE_IN_BYTES, bigarrays);
4540
}
4641

4742
public ReleasableBytesStreamOutput(int expectedSize, BigArrays bigArrays) {
4843
super(expectedSize, bigArrays);
49-
this.releasable = Releasables.releaseOnce(this.bytes);
5044
}
5145

5246
@Override
5347
public void close() {
54-
Releasables.close(releasable);
55-
}
56-
57-
@Override
58-
void ensureCapacity(long offset) {
59-
final ByteArray prevBytes = this.bytes;
60-
super.ensureCapacity(offset);
61-
if (prevBytes != this.bytes) {
62-
// re-create the releasable with the new reference
63-
releasable = Releasables.releaseOnce(this.bytes);
64-
}
65-
}
66-
67-
@Override
68-
public void reset() {
69-
final ByteArray prevBytes = this.bytes;
70-
super.reset();
71-
if (prevBytes != this.bytes) {
72-
// re-create the releasable with the new reference
73-
releasable = Releasables.releaseOnce(this.bytes);
74-
}
48+
bytes.close();
7549
}
7650
}

server/src/main/java/org/elasticsearch/common/lease/Releasables.java

+3-3
Original file line numberDiff line numberDiff line change
@@ -96,13 +96,13 @@ public static Releasable wrap(final Releasable... releasables) {
9696
}
9797

9898
/**
99-
* Equivalent to {@link #wrap(Releasable...)} but can be called multiple times without double releasing.
99+
* Wraps a {@link Releasable} such that its {@link Releasable#close()} method can be called multiple times without double releasing.
100100
*/
101-
public static Releasable releaseOnce(final Releasable... releasables) {
101+
public static Releasable releaseOnce(final Releasable releasable) {
102102
final AtomicBoolean released = new AtomicBoolean(false);
103103
return () -> {
104104
if (released.compareAndSet(false, true)) {
105-
close(releasables);
105+
releasable.close();
106106
}
107107
};
108108
}

server/src/test/java/org/elasticsearch/common/ReleasablesTests.java

+3-3
Original file line numberDiff line numberDiff line change
@@ -28,11 +28,11 @@ public class ReleasablesTests extends ESTestCase {
2828

2929
public void testReleaseOnce() {
3030
AtomicInteger count = new AtomicInteger(0);
31-
Releasable releasable = Releasables.releaseOnce(count::incrementAndGet, count::incrementAndGet);
31+
Releasable releasable = Releasables.releaseOnce(count::incrementAndGet);
3232
assertEquals(0, count.get());
3333
releasable.close();
34-
assertEquals(2, count.get());
34+
assertEquals(1, count.get());
3535
releasable.close();
36-
assertEquals(2, count.get());
36+
assertEquals(1, count.get());
3737
}
3838
}

0 commit comments

Comments
 (0)