Re: [PR] [Netty] Size estimator for ChunkedStream [james-project]

2024-03-22 Thread via GitHub


chibenwa commented on PR #2145:
URL: https://github.com/apache/james-project/pull/2145#issuecomment-2015166917

   See https://github.com/apache/james-project/pull/2149
   
   Instead of inferring the size of the stream, which needs knowledge on the 
fact it is memory based is infered.
   
   Preserving the knowledge that it is memory backed all along the chain, along 
with a tiny fix, enables accurate back pressure.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscr...@james.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


-
To unsubscribe, e-mail: notifications-unsubscr...@james.apache.org
For additional commands, e-mail: notifications-h...@james.apache.org



Re: [PR] [Netty] Size estimator for ChunkedStream [james-project]

2024-03-22 Thread via GitHub


chibenwa closed pull request #2145: [Netty] Size estimator for ChunkedStream
URL: https://github.com/apache/james-project/pull/2145


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscr...@james.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


-
To unsubscribe, e-mail: notifications-unsubscr...@james.apache.org
For additional commands, e-mail: notifications-h...@james.apache.org



Re: [PR] [Netty] Size estimator for ChunkedStream [james-project]

2024-03-22 Thread via GitHub


chibenwa commented on code in PR #2145:
URL: https://github.com/apache/james-project/pull/2145#discussion_r1535378432


##
server/protocols/protocols-imap4/src/main/java/org/apache/james/imapserver/netty/ChunkedStreamWithSize.java:
##
@@ -0,0 +1,150 @@
+/
+ * 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.james.imapserver.netty;
+
+import java.io.InputStream;
+import java.io.PushbackInputStream;
+
+import io.netty.buffer.ByteBuf;
+import io.netty.buffer.ByteBufAllocator;
+import io.netty.channel.ChannelHandlerContext;
+import io.netty.handler.stream.ChunkedInput;
+import io.netty.util.internal.ObjectUtil;
+
+/**
+ * A clone of io.netty.handler.stream.ChunkedStream class, with getChunkSize 
method added
+ */
+public class ChunkedStreamWithSize implements ChunkedInput {
+
+static final int DEFAULT_CHUNK_SIZE = 8192;
+
+private final PushbackInputStream in;
+private final int chunkSize;
+private long offset;
+private boolean closed;
+
+/**
+ * Creates a new instance that fetches data from the specified stream.
+ */
+public ChunkedStreamWithSize(InputStream in) {
+this(in, DEFAULT_CHUNK_SIZE);
+}
+
+/**
+ * Creates a new instance that fetches data from the specified stream.
+ *
+ * @param chunkSize the number of bytes to fetch on each
+ *  {@link #readChunk(ChannelHandlerContext)} call
+ */
+public ChunkedStreamWithSize(InputStream in, int chunkSize) {
+ObjectUtil.checkNotNull(in, "in");
+ObjectUtil.checkPositive(chunkSize, "chunkSize");
+
+if (in instanceof PushbackInputStream) {
+this.in = (PushbackInputStream) in;
+} else {
+this.in = new PushbackInputStream(in);
+}
+this.chunkSize = chunkSize;
+}
+
+/**
+ * Returns the number of transferred bytes.
+ */
+public long transferredBytes() {
+return offset;
+}
+
+@Override
+public boolean isEndOfInput() throws Exception {
+if (closed) {
+return true;
+}
+if (in.available() > 0) {
+return false;
+}
+
+int b = in.read();
+if (b < 0) {
+return true;
+} else {
+in.unread(b);
+return false;
+}
+}
+
+@Override
+public void close() throws Exception {
+closed = true;
+in.close();
+}
+
+@Deprecated
+@Override
+public ByteBuf readChunk(ChannelHandlerContext ctx) throws Exception {
+return readChunk(ctx.alloc());
+}
+
+@Override
+public ByteBuf readChunk(ByteBufAllocator allocator) throws Exception {
+if (isEndOfInput()) {
+return null;
+}
+
+final int availableBytes = in.available();
+final int chunkSize;
+if (availableBytes <= 0) {
+chunkSize = this.chunkSize;
+} else {
+chunkSize = Math.min(this.chunkSize, in.available());
+}
+
+boolean release = true;
+ByteBuf buffer = allocator.buffer(chunkSize);
+try {
+// transfer to buffer
+int written = buffer.writeBytes(in, chunkSize);
+if (written < 0) {
+return null;
+}
+offset += written;
+release = false;
+return buffer;
+} finally {
+if (release) {
+buffer.release();
+}
+}
+}
+
+@Override
+public long length() {
+return -1;
+}
+
+@Override
+public long progress() {
+return offset;
+}
+
+public int getChunkSize() {
+return chunkSize;
+}

Review Comment:
   Well James do actually chucks some kinda transformations of 
"ByteArrayInputStream".
   
  

Re: [PR] [Netty] Size estimator for ChunkedStream [james-project]

2024-03-22 Thread via GitHub


quantranhong1999 commented on PR #2145:
URL: https://github.com/apache/james-project/pull/2145#issuecomment-2014640384

   > With this we could potentially increase watermarks without risking things 
to blow up...
   
   Likely yes.
   I tested with a lower `WriteBufferWaterMark` => more back-pressure 
triggered, while a higher `WriteBufferWaterMark` => less back-pressure 
triggered.
   
   Selecting a big enough `WriteBufferWaterMark` likely would avoid being 
throttled too soon.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscr...@james.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


-
To unsubscribe, e-mail: notifications-unsubscr...@james.apache.org
For additional commands, e-mail: notifications-h...@james.apache.org



Re: [PR] [Netty] Size estimator for ChunkedStream [james-project]

2024-03-21 Thread via GitHub


chibenwa commented on PR #2145:
URL: https://github.com/apache/james-project/pull/2145#issuecomment-2012230199

   Cool.
   
   With this we could potentially increase watermarks without risking things to 
blow up...


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscr...@james.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


-
To unsubscribe, e-mail: notifications-unsubscr...@james.apache.org
For additional commands, e-mail: notifications-h...@james.apache.org