Add a boolean to OpenSSLAeadCipher to control optimisations. #871
Conversation
Controls whether the no-copy optimisations for direct ByteBuffers are enabled. If false, then the inherited default behaciour from CipherSpi is used.
| @@ -223,6 +228,9 @@ boolean allowsNonceReuse() { | |||
| @Override | |||
| protected int engineDoFinal(ByteBuffer input, ByteBuffer output) throws ShortBufferException, | |||
| IllegalBlockSizeException, BadPaddingException { | |||
| if (!ENABLE_BYTEBUFFER_OPTIMIZATIONS) { | |||
| return super.engineDoFinal(input, output); | |||
| } | |||
There was a problem hiding this comment.
From the peanut gallery:
Instead of expensive ByteBuffer.allocateDirect calls, could this method leverage the BufferAllocator? Static state isn't ideal, but using ConscryptEngine.getDefaultBufferAllocator() could reduce overhead. Otherwise, should super.engineDoFinal be used if either buffer is non-direct?
There was a problem hiding this comment.
Thanks!
Absolutely we should reduce overhead further, but this is a starting point and I must admit we only really benchmarked the gRPC case where the buffers are always direct, and where it significantly improves performance.
I'm not sure which would be slower, allocateDirect() and copy or super.engineDoFinal() with its loops of byte array copying and possibly the answer is platform-specific. I guess the code would be simpler at least if we punted everything apart from the happy path to super.
Using BufferAllocator I think would definitely improve things and that's probably the next step, followed by handling the doUpdate() case and possibly other ciphers.
Controls whether the no-copy optimisations for direct ByteBuffers
are enabled. If false, then the inherited default behaciour from
CipherSpi is used.