You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@geode.apache.org by jb...@apache.org on 2021/03/30 19:57:03 UTC
[geode] branch develop updated: GEODE-9081: Remove transient
allocation. (#6207)
This is an automated email from the ASF dual-hosted git repository.
jbarrett pushed a commit to branch develop
in repository https://gitbox.apache.org/repos/asf/geode.git
The following commit(s) were added to refs/heads/develop by this push:
new 7ac9d7e GEODE-9081: Remove transient allocation. (#6207)
7ac9d7e is described below
commit 7ac9d7e4f0d04c99298067ca0611d9326e96d9cf
Author: Jacob Barrett <jb...@pivotal.io>
AuthorDate: Tue Mar 30 12:55:46 2021 -0700
GEODE-9081: Remove transient allocation. (#6207)
* GEODE-9081: Remove transient allocation.
* New geode-unsafe access to non-SDK API for DirectBuffer.
* Use geode-unsafe to access DirectBuffer.attachment().
* Removes latency of reflection.
* Removes transient Object[] allocation in Method.invoke().
---
geode-assembly/build.gradle | 1 +
.../apache/geode/session/tests/TomcatInstall.java | 2 +-
.../org/apache/geode/internal/net/BufferPool.java | 62 ++++++++--------------
.../unsafe/internal/sun/nio/ch/DirectBuffer.java | 36 +++++++++++++
.../internal/sun/nio/ch/DirectBufferTest.java | 47 ++++++++++++++++
5 files changed, 106 insertions(+), 42 deletions(-)
diff --git a/geode-assembly/build.gradle b/geode-assembly/build.gradle
index 8e11e7f..a31d64d 100755
--- a/geode-assembly/build.gradle
+++ b/geode-assembly/build.gradle
@@ -31,6 +31,7 @@ def dependentProjectNames = [
':geode-common',
':geode-connectors',
':geode-core',
+ ':geode-unsafe',
':geode-cq',
':geode-gfsh',
':geode-log4j',
diff --git a/geode-assembly/geode-assembly-test/src/main/java/org/apache/geode/session/tests/TomcatInstall.java b/geode-assembly/geode-assembly-test/src/main/java/org/apache/geode/session/tests/TomcatInstall.java
index c192ed9..2ed7fab 100644
--- a/geode-assembly/geode-assembly-test/src/main/java/org/apache/geode/session/tests/TomcatInstall.java
+++ b/geode-assembly/geode-assembly-test/src/main/java/org/apache/geode/session/tests/TomcatInstall.java
@@ -115,7 +115,7 @@ public class TomcatInstall extends ContainerInstall {
*/
private static final String[] tomcatRequiredJars =
{"antlr", "commons-io", "commons-lang", "commons-validator", "fastutil", "geode-common",
- "geode-core", "geode-deployment-legacy", "geode-log4j", "geode-logging",
+ "geode-core", "geode-unsafe", "geode-deployment-legacy", "geode-log4j", "geode-logging",
"geode-membership", "geode-management", "geode-serialization", "geode-tcp-server",
"javax.transaction-api", "jgroups", "log4j-api", "log4j-core", "log4j-jul", "micrometer",
"shiro-core", "jetty-server", "jetty-util", "jetty-http", "jetty-io"};
diff --git a/geode-core/src/main/java/org/apache/geode/internal/net/BufferPool.java b/geode-core/src/main/java/org/apache/geode/internal/net/BufferPool.java
index 74a2e4d..7a9fccd 100644
--- a/geode-core/src/main/java/org/apache/geode/internal/net/BufferPool.java
+++ b/geode-core/src/main/java/org/apache/geode/internal/net/BufferPool.java
@@ -12,30 +12,25 @@
* or implied. See the License for the specific language governing permissions and limitations under
* the License.
*/
+
package org.apache.geode.internal.net;
import java.lang.ref.SoftReference;
-import java.lang.reflect.Method;
import java.nio.ByteBuffer;
import java.util.IdentityHashMap;
import java.util.concurrent.ConcurrentLinkedQueue;
-import org.apache.logging.log4j.Logger;
-
import org.apache.geode.InternalGemFireException;
import org.apache.geode.annotations.VisibleForTesting;
import org.apache.geode.distributed.internal.DMStats;
import org.apache.geode.distributed.internal.DistributionConfig;
import org.apache.geode.internal.Assert;
import org.apache.geode.internal.tcp.Connection;
-import org.apache.geode.logging.internal.log4j.api.LogService;
+import org.apache.geode.unsafe.internal.sun.nio.ch.DirectBuffer;
import org.apache.geode.util.internal.GeodeGlossary;
public class BufferPool {
private final DMStats stats;
- private static final Logger logger = LogService.getLogger();
-
- private Method parentOfSliceMethod;
/**
* Buffers may be acquired from the Buffers pool
@@ -325,40 +320,25 @@ public class BufferPool {
* If we hand out a buffer that is larger than the requested size we create a
* "slice" of the buffer having the requested capacity and hand that out instead.
* When we put the buffer back in the pool we need to find the original, non-sliced,
- * buffer. This is held in DirectBuffer in its "attachment" field, which is a public
- * method, though DirectBuffer is package-private. This method is visible for use
- * in debugging and testing. For debugging, invoke this method if you need to see
- * the non-sliced buffer for some reason, such as logging its hashcode.
+ * buffer. This is held in DirectBuffer in its "attachment" field.
+ *
+ * This method is visible for use in debugging and testing. For debugging, invoke this method if
+ * you need to see the non-sliced buffer for some reason, such as logging its hashcode.
*/
@VisibleForTesting
- public ByteBuffer getPoolableBuffer(ByteBuffer buffer) {
- if (!buffer.isDirect()) {
+ ByteBuffer getPoolableBuffer(final ByteBuffer buffer) {
+ final Object attachment = DirectBuffer.attachment(buffer);
+
+ if (null == attachment) {
return buffer;
}
- ByteBuffer result = buffer;
- if (parentOfSliceMethod == null) {
- Class clazz = buffer.getClass();
- try {
- Method method = clazz.getMethod("attachment");
- method.setAccessible(true);
- parentOfSliceMethod = method;
- } catch (Exception e) {
- throw new InternalGemFireException("unable to retrieve underlying byte buffer", e);
- }
- }
- try {
- Object attachment = parentOfSliceMethod.invoke(buffer);
- if (attachment instanceof ByteBuffer) {
- result = (ByteBuffer) attachment;
- } else if (attachment != null) {
- throw new InternalGemFireException(
- "direct byte buffer attachment was not a byte buffer but a " +
- attachment.getClass().getName());
- }
- } catch (Exception e) {
- throw new InternalGemFireException("unable to retrieve underlying byte buffer", e);
+
+ if (attachment instanceof ByteBuffer) {
+ return (ByteBuffer) attachment;
}
- return result;
+
+ throw new InternalGemFireException("direct byte buffer attachment was not a byte buffer but a "
+ + attachment.getClass().getName());
}
/**
@@ -383,22 +363,22 @@ public class BufferPool {
BBSoftReference(ByteBuffer bb, boolean send) {
super(bb);
- this.size = bb.capacity();
+ size = bb.capacity();
this.send = send;
}
public int getSize() {
- return this.size;
+ return size;
}
synchronized int consumeSize() {
- int result = this.size;
- this.size = 0;
+ int result = size;
+ size = 0;
return result;
}
public boolean getSend() {
- return this.send;
+ return send;
}
public ByteBuffer getBB() {
diff --git a/geode-unsafe/src/main/java/org/apache/geode/unsafe/internal/sun/nio/ch/DirectBuffer.java b/geode-unsafe/src/main/java/org/apache/geode/unsafe/internal/sun/nio/ch/DirectBuffer.java
new file mode 100644
index 0000000..dc894cf
--- /dev/null
+++ b/geode-unsafe/src/main/java/org/apache/geode/unsafe/internal/sun/nio/ch/DirectBuffer.java
@@ -0,0 +1,36 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more contributor license
+ * agreements. See the NOTICE file distributed with this work for additional information regarding
+ * copyright ownership. The ASF licenses this file to You under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the License. You may obtain a
+ * copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
+ * or implied. See the License for the specific language governing permissions and limitations under
+ * the License.
+ */
+
+package org.apache.geode.unsafe.internal.sun.nio.ch;
+
+/**
+ * Provides access to methods on non-SDK class {@link sun.nio.ch.DirectBuffer}.
+ */
+public interface DirectBuffer {
+
+ /**
+ * @see sun.nio.ch.DirectBuffer#attachment()
+ * @param object to get attachment for
+ * @return returns attachment if object is {@link sun.nio.ch.DirectBuffer} otherwise null.
+ */
+ static Object attachment(final Object object) {
+ if (object instanceof sun.nio.ch.DirectBuffer) {
+ return ((sun.nio.ch.DirectBuffer) object).attachment();
+ }
+
+ return null;
+ }
+
+}
diff --git a/geode-unsafe/src/test/java/org/apache/geode/unsafe/internal/sun/nio/ch/DirectBufferTest.java b/geode-unsafe/src/test/java/org/apache/geode/unsafe/internal/sun/nio/ch/DirectBufferTest.java
new file mode 100644
index 0000000..4a5c51f
--- /dev/null
+++ b/geode-unsafe/src/test/java/org/apache/geode/unsafe/internal/sun/nio/ch/DirectBufferTest.java
@@ -0,0 +1,47 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more contributor license
+ * agreements. See the NOTICE file distributed with this work for additional information regarding
+ * copyright ownership. The ASF licenses this file to You under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the License. You may obtain a
+ * copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
+ * or implied. See the License for the specific language governing permissions and limitations under
+ * the License.
+ */
+
+
+package org.apache.geode.unsafe.internal.sun.nio.ch;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+import java.nio.ByteBuffer;
+
+import org.junit.Test;
+
+public class DirectBufferTest {
+
+ @Test
+ public void attachmentIsNullForNonDirectBuffer() {
+ assertThat(DirectBuffer.attachment(null)).isNull();
+ assertThat(DirectBuffer.attachment(new Object())).isNull();
+ assertThat(DirectBuffer.attachment(ByteBuffer.allocate(1))).isNull();
+ }
+
+ @Test
+ public void attachmentIsNullForUnslicedDirectBuffer() {
+ assertThat(DirectBuffer.attachment(ByteBuffer.allocateDirect(1))).isNull();
+ }
+
+ @Test
+ public void attachmentIsRootBufferForDirectBufferSlice() {
+ final ByteBuffer root = ByteBuffer.allocateDirect(10);
+ final ByteBuffer slice = root.slice();
+
+ assertThat(DirectBuffer.attachment(slice)).isSameAs(root);
+ }
+
+}