From c22166f2efd435799161fad5944953283c467004 Mon Sep 17 00:00:00 2001 From: Ludovic Orban Date: Fri, 27 Oct 2023 12:19:19 +0200 Subject: [PATCH 1/4] #10781 try to stabilize secondary_super_cache Signed-off-by: Ludovic Orban --- .../src/main/java/org/eclipse/jetty/io/FillInterest.java | 4 ++-- .../src/main/java/org/eclipse/jetty/server/Request.java | 6 +++++- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/FillInterest.java b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/FillInterest.java index ec5862ab0731..5a01abc07935 100644 --- a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/FillInterest.java +++ b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/FillInterest.java @@ -19,7 +19,6 @@ import java.util.concurrent.atomic.AtomicReference; import org.eclipse.jetty.util.Callback; -import org.eclipse.jetty.util.thread.Invocable; import org.eclipse.jetty.util.thread.Invocable.InvocationType; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -116,7 +115,8 @@ public boolean isInterested() public InvocationType getCallbackInvocationType() { Callback callback = _interested.get(); - return Invocable.getInvocationType(callback); + // Identical to Invocable.getInvocationType(callback) except that the cast is costly here. + return callback == null ? InvocationType.BLOCKING : callback.getInvocationType(); } /** diff --git a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/Request.java b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/Request.java index 0b955ae9a293..02135c644c3f 100644 --- a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/Request.java +++ b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/Request.java @@ -716,9 +716,12 @@ default InvocationType getInvocationType() */ class Wrapper extends Attributes.Wrapper implements Request { + private final Request request; + public Wrapper(Request wrapped) { super(wrapped); + this.request = wrapped; } @Override @@ -856,7 +859,8 @@ public Session getSession(boolean create) @Override public Request getWrapped() { - return (Request)super.getWrapped(); + // Identical to (Request)super.getWrapped() except that the cast is costly here. + return request; } } From fbb152466b1a2a3e7564639ed667197dc59ad9dc Mon Sep 17 00:00:00 2001 From: Ludovic Orban Date: Mon, 30 Oct 2023 10:01:24 +0100 Subject: [PATCH 2/4] Make Request.Wrapper implement Attributes instead of extending Attributes.Wrapper Signed-off-by: Ludovic Orban --- .../org/eclipse/jetty/server/Request.java | 47 ++++++++++++++++--- 1 file changed, 41 insertions(+), 6 deletions(-) diff --git a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/Request.java b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/Request.java index 02135c644c3f..080012f22b88 100644 --- a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/Request.java +++ b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/Request.java @@ -25,7 +25,9 @@ import java.util.ArrayList; import java.util.List; import java.util.Locale; +import java.util.Map; import java.util.Objects; +import java.util.Set; import java.util.concurrent.CompletableFuture; import java.util.concurrent.TimeoutException; import java.util.function.Consumer; @@ -714,14 +716,13 @@ default InvocationType getInvocationType() /** *

A wrapper for {@code Request} instances.

*/ - class Wrapper extends Attributes.Wrapper implements Request + class Wrapper implements Request, Attributes { - private final Request request; + private final Request wrapped; public Wrapper(Request wrapped) { - super(wrapped); - this.request = wrapped; + this.wrapped = Objects.requireNonNull(wrapped); } @Override @@ -857,10 +858,44 @@ public Session getSession(boolean create) } @Override + public Object removeAttribute(String name) + { + return getWrapped().removeAttribute(name); + } + + @Override + public Object setAttribute(String name, Object attribute) + { + return getWrapped().setAttribute(name, attribute); + } + + @Override + public Object getAttribute(String name) + { + return getWrapped().getAttribute(name); + } + + @Override + public Set getAttributeNameSet() + { + return getWrapped().getAttributeNameSet(); + } + + @Override + public Map asAttributeMap() + { + return getWrapped().asAttributeMap(); + } + + @Override + public void clearAttributes() + { + getWrapped().clearAttributes(); + } + public Request getWrapped() { - // Identical to (Request)super.getWrapped() except that the cast is costly here. - return request; + return wrapped; } } From 346a05f21484377ffe7857cd379f33cd22570ba6 Mon Sep 17 00:00:00 2001 From: Ludovic Orban Date: Mon, 30 Oct 2023 10:19:26 +0100 Subject: [PATCH 3/4] add implementor's note Signed-off-by: Ludovic Orban --- .../java/org/eclipse/jetty/server/Request.java | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/Request.java b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/Request.java index 080012f22b88..205f8ee8795f 100644 --- a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/Request.java +++ b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/Request.java @@ -718,11 +718,19 @@ default InvocationType getInvocationType() */ class Wrapper implements Request, Attributes { - private final Request wrapped; + /** + * Implementation note: Request.Wrapper does not extend from Attributes.Wrapper + * as getWrapped() would either need to be implemented as {@code return (Request)getWrapped()} + * which would requiring a cast from one interface type to another, spoiling the + * secondary_super_cache, or by storing the same {@code _wrapped} object in two fields + * (one in Attributes.Wrapper as type Attributes and one in Request.Wrapper as type Request) + * to save the costly cast. + */ + private final Request _wrapped; public Wrapper(Request wrapped) { - this.wrapped = Objects.requireNonNull(wrapped); + _wrapped = Objects.requireNonNull(wrapped); } @Override @@ -895,7 +903,7 @@ public void clearAttributes() public Request getWrapped() { - return wrapped; + return _wrapped; } } From 2e497141571057ac7f896bf9ca781caef3274f7b Mon Sep 17 00:00:00 2001 From: Ludovic Orban Date: Tue, 31 Oct 2023 09:53:06 +0100 Subject: [PATCH 4/4] improve implementor's note Signed-off-by: Ludovic Orban --- .../main/java/org/eclipse/jetty/server/Request.java | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/Request.java b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/Request.java index 205f8ee8795f..56d7b24b84c6 100644 --- a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/Request.java +++ b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/Request.java @@ -719,12 +719,12 @@ default InvocationType getInvocationType() class Wrapper implements Request, Attributes { /** - * Implementation note: Request.Wrapper does not extend from Attributes.Wrapper - * as getWrapped() would either need to be implemented as {@code return (Request)getWrapped()} - * which would requiring a cast from one interface type to another, spoiling the - * secondary_super_cache, or by storing the same {@code _wrapped} object in two fields - * (one in Attributes.Wrapper as type Attributes and one in Request.Wrapper as type Request) - * to save the costly cast. + * Implementation note: {@link Request.Wrapper} does not extend from {@link Attributes.Wrapper} + * as {@link #getWrapped()} would either need to be implemented as {@code return (Request)getWrapped()} + * which would require a cast from one interface type to another, spoiling the JVM's + * {@code secondary_super_cache}, or by storing the same {@code _wrapped} object in two fields + * (one in {@link Attributes.Wrapper} as type {@link Attributes} and one in {@link Request.Wrapper} as + * type {@link Request}) to save the costly cast from interface type to another. */ private final Request _wrapped;