HADOOP-15499. Performance severe drops when running RawErasureCoderBenchmark with NativeRSRawErasureCoder. Contributed by Sammi Chen.
(cherry picked from commit 18201b882a38ad875358c5d23c09b0ef903c2f91) (cherry picked from commit b8741102758f70e79eb4043b71433560f5ca713e) Project: http://git-wip-us.apache.org/repos/asf/hadoop/repo Commit: http://git-wip-us.apache.org/repos/asf/hadoop/commit/e3c96354 Tree: http://git-wip-us.apache.org/repos/asf/hadoop/tree/e3c96354 Diff: http://git-wip-us.apache.org/repos/asf/hadoop/diff/e3c96354 Branch: refs/remotes/origin/branch-3.1 Commit: e3c96354a749f50038c7604fcc3fb23ecf262add Parents: c0d46a8 Author: Sammi Chen <sammi.c...@intel.com> Authored: Mon Jun 11 13:53:37 2018 +0800 Committer: Sammi Chen <sammi.c...@intel.com> Committed: Mon Jun 11 14:03:39 2018 +0800 ---------------------------------------------------------------------- .../rawcoder/AbstractNativeRawDecoder.java | 51 ++++++++++++-------- .../rawcoder/AbstractNativeRawEncoder.java | 49 +++++++++++-------- .../rawcoder/NativeRSRawDecoder.java | 19 ++++++-- .../rawcoder/NativeRSRawEncoder.java | 19 ++++++-- .../rawcoder/NativeXORRawDecoder.java | 19 ++++++-- .../rawcoder/NativeXORRawEncoder.java | 19 ++++++-- .../rawcoder/RawErasureCoderBenchmark.java | 6 +++ 7 files changed, 127 insertions(+), 55 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/hadoop/blob/e3c96354/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/erasurecode/rawcoder/AbstractNativeRawDecoder.java ---------------------------------------------------------------------- diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/erasurecode/rawcoder/AbstractNativeRawDecoder.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/erasurecode/rawcoder/AbstractNativeRawDecoder.java index e845747..cb71a80 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/erasurecode/rawcoder/AbstractNativeRawDecoder.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/erasurecode/rawcoder/AbstractNativeRawDecoder.java @@ -25,6 +25,7 @@ import org.slf4j.LoggerFactory; import java.io.IOException; import java.nio.ByteBuffer; +import java.util.concurrent.locks.ReentrantReadWriteLock; /** * Abstract native raw decoder for all native coders to extend with. @@ -34,36 +35,46 @@ abstract class AbstractNativeRawDecoder extends RawErasureDecoder { public static Logger LOG = LoggerFactory.getLogger(AbstractNativeRawDecoder.class); + // Protect ISA-L coder data structure in native layer from being accessed and + // updated concurrently by the init, release and decode functions. + protected final ReentrantReadWriteLock decoderLock = + new ReentrantReadWriteLock(); + public AbstractNativeRawDecoder(ErasureCoderOptions coderOptions) { super(coderOptions); } @Override - protected synchronized void doDecode(ByteBufferDecodingState decodingState) + protected void doDecode(ByteBufferDecodingState decodingState) throws IOException { - if (nativeCoder == 0) { - throw new IOException(String.format("%s closed", - getClass().getSimpleName())); - } - int[] inputOffsets = new int[decodingState.inputs.length]; - int[] outputOffsets = new int[decodingState.outputs.length]; + decoderLock.readLock().lock(); + try { + if (nativeCoder == 0) { + throw new IOException(String.format("%s closed", + getClass().getSimpleName())); + } + int[] inputOffsets = new int[decodingState.inputs.length]; + int[] outputOffsets = new int[decodingState.outputs.length]; - ByteBuffer buffer; - for (int i = 0; i < decodingState.inputs.length; ++i) { - buffer = decodingState.inputs[i]; - if (buffer != null) { - inputOffsets[i] = buffer.position(); + ByteBuffer buffer; + for (int i = 0; i < decodingState.inputs.length; ++i) { + buffer = decodingState.inputs[i]; + if (buffer != null) { + inputOffsets[i] = buffer.position(); + } } - } - for (int i = 0; i < decodingState.outputs.length; ++i) { - buffer = decodingState.outputs[i]; - outputOffsets[i] = buffer.position(); - } + for (int i = 0; i < decodingState.outputs.length; ++i) { + buffer = decodingState.outputs[i]; + outputOffsets[i] = buffer.position(); + } - performDecodeImpl(decodingState.inputs, inputOffsets, - decodingState.decodeLength, decodingState.erasedIndexes, - decodingState.outputs, outputOffsets); + performDecodeImpl(decodingState.inputs, inputOffsets, + decodingState.decodeLength, decodingState.erasedIndexes, + decodingState.outputs, outputOffsets); + } finally { + decoderLock.readLock().unlock(); + } } protected abstract void performDecodeImpl(ByteBuffer[] inputs, http://git-wip-us.apache.org/repos/asf/hadoop/blob/e3c96354/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/erasurecode/rawcoder/AbstractNativeRawEncoder.java ---------------------------------------------------------------------- diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/erasurecode/rawcoder/AbstractNativeRawEncoder.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/erasurecode/rawcoder/AbstractNativeRawEncoder.java index cab5383..44d89c2 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/erasurecode/rawcoder/AbstractNativeRawEncoder.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/erasurecode/rawcoder/AbstractNativeRawEncoder.java @@ -25,6 +25,7 @@ import org.slf4j.LoggerFactory; import java.io.IOException; import java.nio.ByteBuffer; +import java.util.concurrent.locks.ReentrantReadWriteLock; /** * Abstract native raw encoder for all native coders to extend with. @@ -34,34 +35,44 @@ abstract class AbstractNativeRawEncoder extends RawErasureEncoder { public static Logger LOG = LoggerFactory.getLogger(AbstractNativeRawEncoder.class); + // Protect ISA-L coder data structure in native layer from being accessed and + // updated concurrently by the init, release and encode functions. + protected final ReentrantReadWriteLock encoderLock = + new ReentrantReadWriteLock(); + public AbstractNativeRawEncoder(ErasureCoderOptions coderOptions) { super(coderOptions); } @Override - protected synchronized void doEncode(ByteBufferEncodingState encodingState) + protected void doEncode(ByteBufferEncodingState encodingState) throws IOException { - if (nativeCoder == 0) { - throw new IOException(String.format("%s closed", - getClass().getSimpleName())); - } - int[] inputOffsets = new int[encodingState.inputs.length]; - int[] outputOffsets = new int[encodingState.outputs.length]; - int dataLen = encodingState.inputs[0].remaining(); + encoderLock.readLock().lock(); + try { + if (nativeCoder == 0) { + throw new IOException(String.format("%s closed", + getClass().getSimpleName())); + } + int[] inputOffsets = new int[encodingState.inputs.length]; + int[] outputOffsets = new int[encodingState.outputs.length]; + int dataLen = encodingState.inputs[0].remaining(); - ByteBuffer buffer; - for (int i = 0; i < encodingState.inputs.length; ++i) { - buffer = encodingState.inputs[i]; - inputOffsets[i] = buffer.position(); - } + ByteBuffer buffer; + for (int i = 0; i < encodingState.inputs.length; ++i) { + buffer = encodingState.inputs[i]; + inputOffsets[i] = buffer.position(); + } - for (int i = 0; i < encodingState.outputs.length; ++i) { - buffer = encodingState.outputs[i]; - outputOffsets[i] = buffer.position(); - } + for (int i = 0; i < encodingState.outputs.length; ++i) { + buffer = encodingState.outputs[i]; + outputOffsets[i] = buffer.position(); + } - performEncodeImpl(encodingState.inputs, inputOffsets, dataLen, - encodingState.outputs, outputOffsets); + performEncodeImpl(encodingState.inputs, inputOffsets, dataLen, + encodingState.outputs, outputOffsets); + } finally { + encoderLock.readLock().unlock(); + } } protected abstract void performEncodeImpl( http://git-wip-us.apache.org/repos/asf/hadoop/blob/e3c96354/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/erasurecode/rawcoder/NativeRSRawDecoder.java ---------------------------------------------------------------------- diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/erasurecode/rawcoder/NativeRSRawDecoder.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/erasurecode/rawcoder/NativeRSRawDecoder.java index 8572222..dc2c33a 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/erasurecode/rawcoder/NativeRSRawDecoder.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/erasurecode/rawcoder/NativeRSRawDecoder.java @@ -36,19 +36,30 @@ public class NativeRSRawDecoder extends AbstractNativeRawDecoder { public NativeRSRawDecoder(ErasureCoderOptions coderOptions) { super(coderOptions); - initImpl(coderOptions.getNumDataUnits(), coderOptions.getNumParityUnits()); + decoderLock.writeLock().lock(); + try { + initImpl(coderOptions.getNumDataUnits(), + coderOptions.getNumParityUnits()); + } finally { + decoderLock.writeLock().unlock(); + } } @Override - protected synchronized void performDecodeImpl( + protected void performDecodeImpl( ByteBuffer[] inputs, int[] inputOffsets, int dataLen, int[] erased, ByteBuffer[] outputs, int[] outputOffsets) throws IOException { decodeImpl(inputs, inputOffsets, dataLen, erased, outputs, outputOffsets); } @Override - public synchronized void release() { - destroyImpl(); + public void release() { + decoderLock.writeLock().lock(); + try { + destroyImpl(); + } finally { + decoderLock.writeLock().unlock(); + } } @Override http://git-wip-us.apache.org/repos/asf/hadoop/blob/e3c96354/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/erasurecode/rawcoder/NativeRSRawEncoder.java ---------------------------------------------------------------------- diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/erasurecode/rawcoder/NativeRSRawEncoder.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/erasurecode/rawcoder/NativeRSRawEncoder.java index 754ec88..ad06927 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/erasurecode/rawcoder/NativeRSRawEncoder.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/erasurecode/rawcoder/NativeRSRawEncoder.java @@ -36,19 +36,30 @@ public class NativeRSRawEncoder extends AbstractNativeRawEncoder { public NativeRSRawEncoder(ErasureCoderOptions coderOptions) { super(coderOptions); - initImpl(coderOptions.getNumDataUnits(), coderOptions.getNumParityUnits()); + encoderLock.writeLock().lock(); + try { + initImpl(coderOptions.getNumDataUnits(), + coderOptions.getNumParityUnits()); + } finally { + encoderLock.writeLock().unlock(); + } } @Override - protected synchronized void performEncodeImpl( + protected void performEncodeImpl( ByteBuffer[] inputs, int[] inputOffsets, int dataLen, ByteBuffer[] outputs, int[] outputOffsets) throws IOException { encodeImpl(inputs, inputOffsets, dataLen, outputs, outputOffsets); } @Override - public synchronized void release() { - destroyImpl(); + public void release() { + encoderLock.writeLock().lock(); + try { + destroyImpl(); + } finally { + encoderLock.writeLock().unlock(); + } } @Override http://git-wip-us.apache.org/repos/asf/hadoop/blob/e3c96354/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/erasurecode/rawcoder/NativeXORRawDecoder.java ---------------------------------------------------------------------- diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/erasurecode/rawcoder/NativeXORRawDecoder.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/erasurecode/rawcoder/NativeXORRawDecoder.java index 1763042..dd708eb 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/erasurecode/rawcoder/NativeXORRawDecoder.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/erasurecode/rawcoder/NativeXORRawDecoder.java @@ -36,19 +36,30 @@ public class NativeXORRawDecoder extends AbstractNativeRawDecoder { public NativeXORRawDecoder(ErasureCoderOptions coderOptions) { super(coderOptions); - initImpl(coderOptions.getNumDataUnits(), coderOptions.getNumParityUnits()); + decoderLock.writeLock().lock(); + try { + initImpl(coderOptions.getNumDataUnits(), + coderOptions.getNumParityUnits()); + } finally { + decoderLock.writeLock().unlock(); + } } @Override - protected synchronized void performDecodeImpl( + protected void performDecodeImpl( ByteBuffer[] inputs, int[] inputOffsets, int dataLen, int[] erased, ByteBuffer[] outputs, int[] outputOffsets) throws IOException { decodeImpl(inputs, inputOffsets, dataLen, erased, outputs, outputOffsets); } @Override - public synchronized void release() { - destroyImpl(); + public void release() { + decoderLock.writeLock().lock(); + try { + destroyImpl(); + } finally { + decoderLock.writeLock().unlock(); + } } private native void initImpl(int numDataUnits, int numParityUnits); http://git-wip-us.apache.org/repos/asf/hadoop/blob/e3c96354/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/erasurecode/rawcoder/NativeXORRawEncoder.java ---------------------------------------------------------------------- diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/erasurecode/rawcoder/NativeXORRawEncoder.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/erasurecode/rawcoder/NativeXORRawEncoder.java index 7f4265b..66b0a1b 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/erasurecode/rawcoder/NativeXORRawEncoder.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/erasurecode/rawcoder/NativeXORRawEncoder.java @@ -36,19 +36,30 @@ public class NativeXORRawEncoder extends AbstractNativeRawEncoder { public NativeXORRawEncoder(ErasureCoderOptions coderOptions) { super(coderOptions); - initImpl(coderOptions.getNumDataUnits(), coderOptions.getNumParityUnits()); + encoderLock.writeLock().lock(); + try { + initImpl(coderOptions.getNumDataUnits(), + coderOptions.getNumParityUnits()); + } finally { + encoderLock.writeLock().unlock(); + } } @Override - protected synchronized void performEncodeImpl( + protected void performEncodeImpl( ByteBuffer[] inputs, int[] inputOffsets, int dataLen, ByteBuffer[] outputs, int[] outputOffsets) throws IOException { encodeImpl(inputs, inputOffsets, dataLen, outputs, outputOffsets); } @Override - public synchronized void release() { - destroyImpl(); + public void release() { + encoderLock.writeLock().lock(); + try { + destroyImpl(); + } finally { + encoderLock.writeLock().unlock(); + } } private native void initImpl(int numDataUnits, int numParityUnits); http://git-wip-us.apache.org/repos/asf/hadoop/blob/e3c96354/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/io/erasurecode/rawcoder/RawErasureCoderBenchmark.java ---------------------------------------------------------------------- diff --git a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/io/erasurecode/rawcoder/RawErasureCoderBenchmark.java b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/io/erasurecode/rawcoder/RawErasureCoderBenchmark.java index c005e77..df8c54b 100644 --- a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/io/erasurecode/rawcoder/RawErasureCoderBenchmark.java +++ b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/io/erasurecode/rawcoder/RawErasureCoderBenchmark.java @@ -230,6 +230,12 @@ public final class RawErasureCoderBenchmark { throw e; } finally { executor.shutdown(); + if (encoder != null) { + encoder.release(); + } + if (decoder != null) { + decoder.release(); + } } } --------------------------------------------------------------------- To unsubscribe, e-mail: common-commits-unsubscr...@hadoop.apache.org For additional commands, e-mail: common-commits-h...@hadoop.apache.org
