Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fixes #2828 - AbstractHTTP2ServerConnectionFactory concurrent connect low performance. #2831

Merged
merged 2 commits into from
Aug 22, 2018
Merged

Fixes #2828 - AbstractHTTP2ServerConnectionFactory concurrent connect low performance. #2831

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d3e75c5
Fixes #2828 - AbstractHTTP2ServerConnectionFactory concurrent connect…
sbordet Aug 19, 2018
97a2f3b
Fixes #2828 - AbstractHTTP2ServerConnectionFactory concurrent connect…
sbordet Aug 21, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 44 additions & 5 deletions ...er/src/main/java/org/eclipse/jetty/http2/server/AbstractHTTP2ServerConnectionFactory.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,17 @@

package org.eclipse.jetty.http2.server;

import java.io.IOException;
import java.util.HashMap;
import java.util.Map;
import java.util.Objects;
import java.util.Set;
import java.util.concurrent.ConcurrentHashMap;

import org.eclipse.jetty.http2.BufferingFlowControlStrategy;
import org.eclipse.jetty.http2.FlowControlStrategy;
import org.eclipse.jetty.http2.HTTP2Connection;
import org.eclipse.jetty.http2.api.Session;
import org.eclipse.jetty.http2.api.server.ServerSessionListener;
import org.eclipse.jetty.http2.frames.Frame;
import org.eclipse.jetty.http2.frames.SettingsFrame;
Expand All @@ -38,12 +42,14 @@
import org.eclipse.jetty.util.annotation.ManagedAttribute;
import org.eclipse.jetty.util.annotation.ManagedObject;
import org.eclipse.jetty.util.annotation.Name;
import org.eclipse.jetty.util.component.ContainerLifeCycle;
import org.eclipse.jetty.util.component.Dumpable;
import org.eclipse.jetty.util.component.LifeCycle;

@ManagedObject
public abstract class AbstractHTTP2ServerConnectionFactory extends AbstractConnectionFactory
{
private final Connection.Listener connectionListener = new ConnectionListener();
private final HTTP2SessionContainer sessionContainer = new HTTP2SessionContainer();
private final HttpConfiguration httpConfiguration;
private int maxDynamicTableSize = 4096;
private int initialSessionRecvWindow = 1024 * 1024;
Expand All @@ -66,6 +72,7 @@ protected AbstractHTTP2ServerConnectionFactory(@Name("config") HttpConfiguration
for (String p:protocols)
if (!HTTP2ServerConnection.isSupportedProtocol(p))
throw new IllegalArgumentException("Unsupported HTTP2 Protocol variant: "+p);
addBean(sessionContainer);
this.httpConfiguration = Objects.requireNonNull(httpConfiguration);
addBean(httpConfiguration);
setInputBufferSize(Frame.DEFAULT_MAX_LENGTH + Frame.HEADER_LENGTH);
Expand Down Expand Up @@ -234,7 +241,7 @@ public Connection newConnection(Connector connector, EndPoint endPoint)

HTTP2Connection connection = new HTTP2ServerConnection(connector.getByteBufferPool(), connector.getExecutor(),
endPoint, httpConfiguration, parser, session, getInputBufferSize(), listener);
connection.addListener(connectionListener);
connection.addListener(sessionContainer);
return configure(connection, connector, endPoint);
}

Expand All @@ -245,18 +252,50 @@ protected ServerParser newServerParser(Connector connector, ServerParser.Listene
return new ServerParser(connector.getByteBufferPool(), listener, getMaxDynamicTableSize(), getHttpConfiguration().getRequestHeaderSize());
}

private class ConnectionListener implements Connection.Listener
@ManagedObject("The container of HTTP/2 sessions")
public static class HTTP2SessionContainer implements Connection.Listener, Dumpable
{
private final Set<Session> sessions = ConcurrentHashMap.newKeySet();

@Override
public void onOpened(Connection connection)
{
addManaged((LifeCycle)((HTTP2Connection)connection).getSession());
Session session = ((HTTP2Connection)connection).getSession();
sessions.add(session);
LifeCycle.start(session);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if start throws an exception, will onClosed be called to remove it?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Historically, if an exception was thrown from a Connection.Listener we let it unwind the stack, which I don't think it's ideal.
I think such listeners should be wrapped in try/catch, log the exception and continue.
So either we wrap, or we leave it as is. I'm for wrapping.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

log and continue

}

@Override
public void onClosed(Connection connection)
{
removeBean(((HTTP2Connection)connection).getSession());
Session session = ((HTTP2Connection)connection).getSession();
if (sessions.remove(session))
LifeCycle.stop(session);
}

@ManagedAttribute(value = "The number of HTTP/2 sessions", readonly = true)
public int getSize()
{
return sessions.size();
}

@Override
public String dump()
{
return ContainerLifeCycle.dump(this);
}

@Override
public void dump(Appendable out, String indent) throws IOException
{
ContainerLifeCycle.dumpObject(out, this);
ContainerLifeCycle.dump(out, indent, sessions);
}

@Override
public String toString()
{
return String.format("%s@%x[size=%d]", getClass().getSimpleName(), hashCode(), sessions.size());
}
}
}