diff options
author | Raymond Auge | 2017-01-09 18:52:35 +0000 |
---|---|---|
committer | Raymond Auge | 2017-01-09 18:52:35 +0000 |
commit | de7301522786b873b7689e613feeba768ab91040 (patch) | |
tree | a0435ead234e6321426071a6f840c6e8585c5ffc | |
parent | fc458dd083d984d1d217142b76234e7029ccf02f (diff) | |
download | rt.equinox.bundles-de7301522786b873b7689e613feeba768ab91040.tar.gz rt.equinox.bundles-de7301522786b873b7689e613feeba768ab91040.tar.xz rt.equinox.bundles-de7301522786b873b7689e613feeba768ab91040.zip |
bug 509100 - [http servlet] performance tuning
LPS-67139 Defer toString() value calculation
LPS-69622 Dead field, never initialized, never used.
LPS-69622 Use ConcurrentHashMap rather than synchronized all methods.
LPS-69622 Use AtomicInteger to prevent lost counting.
LPS-69622 Fallback to JDK6 only api. To ensure thread safety, createContextAttributes()/destroyContextAttributes() must still have the synchronized protection, consider they are only used during application initialization/destroy, there is no runtime overhead. At least this way getContextAttributes() is synchronized free, which is the most important thing to have.
LPS-69622 Avoid using Hashtable, it just needs to be a Dictionary, delegate to ConcurrentHashMap
LPS-69880 Avoid creating HttpSessionEvent unless there is any HttpSessionListener registered
LPS-69882 Avoid duplicated path string indexing and splitting
LPS-69883 The only creation of HttpServiceRuntimeImpl in Activator.addingService() is already wrapping the serviceProperties with UMDictionaryMap, inside HttpServiceRuntimeImpl constructor, there is no need to wrap it.
LPS-69903 Shortcut as early as possible
LPS-69903 Avoid using java.util.Stack, use LinkedList instead
Signed-off-by: shuyangzhou <shuyang.zhou@liferay.com>
Signed-off-by: Matthew Tambara <matthew.tambara@liferay.com>
Signed-off-by: Raymond Auge <raymond.auge@liferay.com>
8 files changed, 178 insertions, 87 deletions
diff --git a/bundles/org.eclipse.equinox.http.servlet/src/org/eclipse/equinox/http/servlet/internal/HttpServiceRuntimeImpl.java b/bundles/org.eclipse.equinox.http.servlet/src/org/eclipse/equinox/http/servlet/internal/HttpServiceRuntimeImpl.java index 9d39f3e98..62474bcef 100644 --- a/bundles/org.eclipse.equinox.http.servlet/src/org/eclipse/equinox/http/servlet/internal/HttpServiceRuntimeImpl.java +++ b/bundles/org.eclipse.equinox.http.servlet/src/org/eclipse/equinox/http/servlet/internal/HttpServiceRuntimeImpl.java @@ -60,7 +60,7 @@ public class HttpServiceRuntimeImpl this.listenerServiceFilter = createListenerFilter(consumingContext, parentServletContext); this.parentServletContext = parentServletContext; - this.attributes = Collections.unmodifiableMap(attributes); + this.attributes = attributes; this.targetFilter = "(" + Activator.UNIQUE_SERVICE_ID + "=" + attributes.get(Activator.UNIQUE_SERVICE_ID) + ")"; //$NON-NLS-1$ //$NON-NLS-2$ //$NON-NLS-3$ contextServiceTracker = @@ -199,10 +199,12 @@ public class HttpServiceRuntimeImpl } public DispatchTargets getDispatchTargets( - String path, RequestInfoDTO requestInfoDTO) { + String pathString, RequestInfoDTO requestInfoDTO) { - String queryString = Path.findQueryString(path); - String requestURI = Path.stripQueryString(path); + Path path = new Path(pathString); + + String queryString = path.getQueryString(); + String requestURI = path.getRequestURI(); // perfect match DispatchTargets dispatchTargets = getDispatchTargets( @@ -210,10 +212,9 @@ public class HttpServiceRuntimeImpl if (dispatchTargets == null) { // extension match - String extension = Path.findExtension(requestURI); dispatchTargets = getDispatchTargets( - requestURI, extension, queryString, Match.EXTENSION, + requestURI, path.getExtension(), queryString, Match.EXTENSION, requestInfoDTO); } diff --git a/bundles/org.eclipse.equinox.http.servlet/src/org/eclipse/equinox/http/servlet/internal/context/ContextController.java b/bundles/org.eclipse.equinox.http.servlet/src/org/eclipse/equinox/http/servlet/internal/context/ContextController.java index 2a15d8a4a..a2d4a7227 100644 --- a/bundles/org.eclipse.equinox.http.servlet/src/org/eclipse/equinox/http/servlet/internal/context/ContextController.java +++ b/bundles/org.eclipse.equinox.http.servlet/src/org/eclipse/equinox/http/servlet/internal/context/ContextController.java @@ -144,8 +144,6 @@ public class ContextController { this.trackingContext = trackingContextParam; this.consumingContext = consumingContext; - this.string = SIMPLE_NAME + '[' + serviceId + "][" + contextName + ", " + trackingContextParam.getBundle() + ']'; //$NON-NLS-1$ - listenerServiceTracker = new ServiceTracker<EventListener, AtomicReference<ListenerRegistration>>( trackingContext, httpServiceRuntime.getListenerFilter(), new ContextListenerTrackerCustomizer( @@ -639,10 +637,12 @@ public class ContextController { } public DispatchTargets getDispatchTargets( - String path, RequestInfoDTO requestInfoDTO) { + String pathString, RequestInfoDTO requestInfoDTO) { + + Path path = new Path(pathString); - String queryString = Path.findQueryString(path); - String requestURI = Path.stripQueryString(path); + String queryString = path.getQueryString(); + String requestURI = path.getRequestURI(); // perfect match DispatchTargets dispatchTargets = getDispatchTargets( @@ -650,10 +650,9 @@ public class ContextController { if (dispatchTargets == null) { // extension match - String extension = Path.findExtension(requestURI); dispatchTargets = getDispatchTargets( - requestURI, extension, queryString, Match.EXTENSION, + requestURI, path.getExtension(), queryString, Match.EXTENSION, requestInfoDTO); } @@ -926,7 +925,15 @@ public class ContextController { @Override public String toString() { - return string; + String value = string; + + if (value == null) { + value = SIMPLE_NAME + '[' + contextName + ", " + trackingContext.getBundle() + ']'; //$NON-NLS-1$ + + string = value; + } + + return value; } private void addEnpointRegistrationsToRequestInfo( @@ -1241,12 +1248,16 @@ public class ContextController { return previousHttpSessionAdaptor; } + List<HttpSessionListener> listeners = eventListeners.get(HttpSessionListener.class); + + if (listeners.isEmpty()) { + return httpSessionAdaptor; + } + HttpSessionEvent httpSessionEvent = new HttpSessionEvent( httpSessionAdaptor); - for (HttpSessionListener listener : eventListeners.get( - HttpSessionListener.class)) { - + for (HttpSessionListener listener : listeners) { listener.sessionCreated(httpSessionEvent); } @@ -1295,7 +1306,7 @@ public class ContextController { private final ServiceReference<ServletContextHelper> servletContextHelperRef; private final String servletContextHelperRefFilter; private boolean shutdown; - private final String string; + private String string; private final ServiceTracker<Filter, AtomicReference<FilterRegistration>> filterServiceTracker; private final ServiceTracker<EventListener, AtomicReference<ListenerRegistration>> listenerServiceTracker; diff --git a/bundles/org.eclipse.equinox.http.servlet/src/org/eclipse/equinox/http/servlet/internal/context/ProxyContext.java b/bundles/org.eclipse.equinox.http.servlet/src/org/eclipse/equinox/http/servlet/internal/context/ProxyContext.java index d92bc13f8..14a14d477 100644 --- a/bundles/org.eclipse.equinox.http.servlet/src/org/eclipse/equinox/http/servlet/internal/context/ProxyContext.java +++ b/bundles/org.eclipse.equinox.http.servlet/src/org/eclipse/equinox/http/servlet/internal/context/ProxyContext.java @@ -13,7 +13,11 @@ package org.eclipse.equinox.http.servlet.internal.context; import java.io.File; +import java.io.Serializable; import java.util.*; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.ConcurrentMap; +import java.util.concurrent.atomic.AtomicInteger; import javax.servlet.ServletContext; import org.eclipse.equinox.http.servlet.internal.util.Const; @@ -28,9 +32,8 @@ import org.eclipse.equinox.http.servlet.internal.util.Const; public class ProxyContext { private static final String JAVAX_SERVLET_CONTEXT_TEMPDIR = "javax.servlet.context.tempdir"; //$NON-NLS-1$ - private String servletPath; - private HashMap<ContextController, ContextAttributes> attributesMap = - new HashMap<ContextController, ContextAttributes>(); + private final ConcurrentMap<ContextController, ContextAttributes> attributesMap = + new ConcurrentHashMap<ContextController, ContextAttributes>(); File proxyContextTempDir; private ServletContext servletContext; @@ -49,36 +52,44 @@ public class ProxyContext { deleteDirectory(proxyContextTempDir); } - public synchronized String getServletPath() { - return (servletPath == null ? Const.BLANK : servletPath); + public String getServletPath() { + return Const.BLANK; } - public synchronized void createContextAttributes( + public void createContextAttributes( ContextController controller) { - ContextAttributes attributes = attributesMap.get(controller); - if (attributes == null) { - attributes = new ContextAttributes(controller); - attributesMap.put(controller, attributes); + synchronized (attributesMap) { + ContextAttributes contextAttributes = attributesMap.get(controller); + + if (contextAttributes == null) { + contextAttributes = new ContextAttributes(controller); + + attributesMap.put(controller, contextAttributes); + } + + contextAttributes.addReference(); } - attributes.addReference(); } - public synchronized void destroyContextAttributes( + public void destroyContextAttributes( ContextController controller) { - ContextAttributes attributes = attributesMap.get(controller); - if (attributes == null) { - throw new IllegalStateException("too many calls"); - } - attributes.removeReference(); - if (attributes.referenceCount() == 0) { - attributesMap.remove(controller); - attributes.destroy(); + synchronized (attributesMap) { + ContextAttributes contextAttributes = attributesMap.get(controller); + + if (contextAttributes == null) { + throw new IllegalStateException("too many calls"); + } + + if (contextAttributes.removeReference() == 0) { + attributesMap.remove(controller); + contextAttributes.destroy(); + } } } - public synchronized Dictionary<String, Object> getContextAttributes( + public Dictionary<String, Object> getContextAttributes( ContextController controller) { return attributesMap.get(controller); @@ -107,9 +118,11 @@ public class ProxyContext { return directory.delete(); } - public class ContextAttributes extends Hashtable<String, Object> { + public class ContextAttributes + extends Dictionary<String, Object> implements Serializable { + private static final long serialVersionUID = 1916670423277243587L; - private int referenceCount; + private final AtomicInteger referenceCount = new AtomicInteger(); public ContextAttributes(ContextController controller) { if (proxyContextTempDir != null) { @@ -129,17 +142,55 @@ public class ProxyContext { deleteDirectory(contextTempDir); } - public void addReference() { - referenceCount++; + public int addReference() { + return referenceCount.incrementAndGet(); } - public void removeReference() { - referenceCount--; + public int removeReference() { + return referenceCount.decrementAndGet(); } public int referenceCount() { - return referenceCount; + return referenceCount.get(); } + + @Override + public Enumeration<Object> elements() { + return Collections.enumeration(_map.values()); + } + + @Override + public Object get(Object key) { + return _map.get(key); + } + + @Override + public boolean isEmpty() { + return _map.isEmpty(); + } + + @Override + public Enumeration<String> keys() { + return Collections.enumeration(_map.keySet()); + } + + @Override + public Object put(String key, Object value) { + return _map.put(key, value); + } + + @Override + public Object remove(Object key) { + return _map.remove(key); + } + + @Override + public int size() { + return _map.size(); + } + + private final Map<String, Object> _map = + new ConcurrentHashMap<String, Object>(); } } diff --git a/bundles/org.eclipse.equinox.http.servlet/src/org/eclipse/equinox/http/servlet/internal/servlet/HttpServletRequestWrapperImpl.java b/bundles/org.eclipse.equinox.http.servlet/src/org/eclipse/equinox/http/servlet/internal/servlet/HttpServletRequestWrapperImpl.java index 6472681f1..350ef26ea 100644 --- a/bundles/org.eclipse.equinox.http.servlet/src/org/eclipse/equinox/http/servlet/internal/servlet/HttpServletRequestWrapperImpl.java +++ b/bundles/org.eclipse.equinox.http.servlet/src/org/eclipse/equinox/http/servlet/internal/servlet/HttpServletRequestWrapperImpl.java @@ -27,7 +27,7 @@ import org.osgi.service.http.HttpContext; public class HttpServletRequestWrapperImpl extends HttpServletRequestWrapper { - private final Stack<DispatchTargets> dispatchTargets = new Stack<DispatchTargets>(); + private final Deque<DispatchTargets> dispatchTargets = new LinkedList<DispatchTargets>(); private final HttpServletRequest request; private Map<String, Part> parts; private final Lock lock = new ReentrantLock(); @@ -93,7 +93,7 @@ public class HttpServletRequestWrapperImpl extends HttpServletRequestWrapper { if ((currentDispatchTargets.getServletName() != null) || (currentDispatchTargets.getDispatcherType() == DispatcherType.INCLUDE)) { - return this.dispatchTargets.get(0).getPathInfo(); + return this.dispatchTargets.getLast().getPathInfo(); } return currentDispatchTargets.getPathInfo(); } @@ -153,7 +153,7 @@ public class HttpServletRequestWrapperImpl extends HttpServletRequestWrapper { if ((currentDispatchTargets.getServletName() != null) || (currentDispatchTargets.getDispatcherType() == DispatcherType.INCLUDE)) { - return this.dispatchTargets.get(0).getServletPath(); + return this.dispatchTargets.getLast().getServletPath(); } if (currentDispatchTargets.getServletPath().equals(Const.SLASH)) { return Const.BLANK; @@ -168,6 +168,14 @@ public class HttpServletRequestWrapperImpl extends HttpServletRequestWrapper { public Object getAttribute(String attributeName) { DispatchTargets current = dispatchTargets.peek(); DispatcherType dispatcherType = current.getDispatcherType(); + + if ((dispatcherType == DispatcherType.ASYNC) || + (dispatcherType == DispatcherType.REQUEST) || + !attributeName.startsWith("javax.servlet.")) { + + return request.getAttribute(attributeName); + } + boolean hasServletName = (current.getServletName() != null); Map<String, Object> specialOverides = current.getSpecialOverides(); @@ -234,7 +242,7 @@ public class HttpServletRequestWrapperImpl extends HttpServletRequestWrapper { return null; } - DispatchTargets original = dispatchTargets.get(0); + DispatchTargets original = dispatchTargets.getLast(); if (attributeName.equals(RequestDispatcher.FORWARD_CONTEXT_PATH)) { if (specialOverides.containsKey(RequestDispatcher.FORWARD_CONTEXT_PATH)) { diff --git a/bundles/org.eclipse.equinox.http.servlet/src/org/eclipse/equinox/http/servlet/internal/servlet/HttpSessionAdaptor.java b/bundles/org.eclipse.equinox.http.servlet/src/org/eclipse/equinox/http/servlet/internal/servlet/HttpSessionAdaptor.java index 58ca6e53e..6e6b27c1c 100644 --- a/bundles/org.eclipse.equinox.http.servlet/src/org/eclipse/equinox/http/servlet/internal/servlet/HttpSessionAdaptor.java +++ b/bundles/org.eclipse.equinox.http.servlet/src/org/eclipse/equinox/http/servlet/internal/servlet/HttpSessionAdaptor.java @@ -178,7 +178,7 @@ public class HttpSessionAdaptor implements HttpSession, Serializable { private transient final HttpSession session; private transient final ServletContext servletContext; private transient final String attributePrefix; - private final String string; + private String string; static public HttpSessionAdaptor createHttpSessionAdaptor( HttpSession session, ServletContext servletContext, ContextController controller) { @@ -194,8 +194,6 @@ public class HttpSessionAdaptor implements HttpSession, Serializable { this.servletContext = servletContext; this.controller = controller; this.attributePrefix = "equinox.http." + controller.getContextName(); //$NON-NLS-1$ - - this.string = SIMPLE_NAME + '[' + session.getId() + ", " + attributePrefix + ']'; //$NON-NLS-1$ } public ContextController getController() { @@ -338,7 +336,15 @@ public class HttpSessionAdaptor implements HttpSession, Serializable { @Override public String toString() { - return string; + String value = string; + + if (value == null) { + value = SIMPLE_NAME + '[' + session.getId() + ", " + attributePrefix + ']'; //$NON-NLS-1$ + + string = value; + } + + return value; } private static final String SIMPLE_NAME = diff --git a/bundles/org.eclipse.equinox.http.servlet/src/org/eclipse/equinox/http/servlet/internal/servlet/RequestDispatcherAdaptor.java b/bundles/org.eclipse.equinox.http.servlet/src/org/eclipse/equinox/http/servlet/internal/servlet/RequestDispatcherAdaptor.java index c07d8189e..7ce012943 100644 --- a/bundles/org.eclipse.equinox.http.servlet/src/org/eclipse/equinox/http/servlet/internal/servlet/RequestDispatcherAdaptor.java +++ b/bundles/org.eclipse.equinox.http.servlet/src/org/eclipse/equinox/http/servlet/internal/servlet/RequestDispatcherAdaptor.java @@ -25,15 +25,13 @@ public class RequestDispatcherAdaptor implements RequestDispatcher { private final DispatchTargets dispatchTargets; private final String path; - private final String string; + private String string; public RequestDispatcherAdaptor( DispatchTargets dispatchTargets, String path) { this.dispatchTargets = dispatchTargets; this.path = path; - - this.string = SIMPLE_NAME + '[' + path + ", " + dispatchTargets + ']'; //$NON-NLS-1$ } public void forward(ServletRequest request, ServletResponse response) @@ -54,7 +52,15 @@ public class RequestDispatcherAdaptor implements RequestDispatcher { @Override public String toString() { - return string; + String value = string; + + if (value == null) { + value = SIMPLE_NAME + '[' + path + ", " + dispatchTargets + ']'; //$NON-NLS-1$ + + string = value; + } + + return value; } } diff --git a/bundles/org.eclipse.equinox.http.servlet/src/org/eclipse/equinox/http/servlet/internal/servlet/ServletContextAdaptor.java b/bundles/org.eclipse.equinox.http.servlet/src/org/eclipse/equinox/http/servlet/internal/servlet/ServletContextAdaptor.java index 35a87d47a..2a627fc62 100644 --- a/bundles/org.eclipse.equinox.http.servlet/src/org/eclipse/equinox/http/servlet/internal/servlet/ServletContextAdaptor.java +++ b/bundles/org.eclipse.equinox.http.servlet/src/org/eclipse/equinox/http/servlet/internal/servlet/ServletContextAdaptor.java @@ -87,8 +87,6 @@ public class ServletContextAdaptor { BundleWiring bundleWiring = this.bundle.adapt(BundleWiring.class); this.classLoader = bundleWiring.getClassLoader(); - - this.string = SIMPLE_NAME + '[' + contextController + ']'; } public ServletContext createServletContext() { @@ -381,7 +379,15 @@ public class ServletContextAdaptor { } public String toString() { - return string; + String value = string; + + if (value == null) { + value = SIMPLE_NAME + '[' + contextController + ']'; + + string = value; + } + + return value; } Object invoke(Object proxy, Method method, Object[] args) throws Throwable { @@ -443,6 +449,6 @@ public class ServletContextAdaptor { private final ProxyContext proxyContext; private final ServletContext servletContext; final ServletContextHelper servletContextHelper; - private final String string; + private String string; } diff --git a/bundles/org.eclipse.equinox.http.servlet/src/org/eclipse/equinox/http/servlet/internal/util/Path.java b/bundles/org.eclipse.equinox.http.servlet/src/org/eclipse/equinox/http/servlet/internal/util/Path.java index 70e57a1c4..dc462bcbc 100644 --- a/bundles/org.eclipse.equinox.http.servlet/src/org/eclipse/equinox/http/servlet/internal/util/Path.java +++ b/bundles/org.eclipse.equinox.http.servlet/src/org/eclipse/equinox/http/servlet/internal/util/Path.java @@ -16,40 +16,42 @@ package org.eclipse.equinox.http.servlet.internal.util; */ public class Path { - public static String findExtension(String path) { - path = stripQueryString(path); - - String lastSegment = path.substring(path.lastIndexOf('/') + 1); - - int dot = lastSegment.lastIndexOf('.'); + public Path(String path) { + int index = path.indexOf('?'); - if (dot == -1) { - return null; + if (index == -1) { + _requestURI = path; + _queryString = null; + } + else { + _requestURI = path.substring(0, index); + _queryString = path.substring(index + 1); } - return lastSegment.substring(dot + 1); - } - - public static String findQueryString(String path) { - String queryString = null; - - int index = path.indexOf('?'); + index = _requestURI.lastIndexOf('.'); - if (index != -1) { - queryString = path.substring(index + 1); + if ((index == -1) || (index < _requestURI.lastIndexOf('/'))) { + _extension = null; + } + else { + _extension = _requestURI.substring(index + 1); } - - return queryString; } - public static String stripQueryString(String path) { - int index = path.indexOf('?'); + public String getRequestURI() { + return _requestURI; + } - if (index != -1) { - path = path.substring(0, index); - } + public String getQueryString() { + return _queryString; + } - return path; + public String getExtension() { + return _extension; } + private final String _requestURI; + private final String _queryString; + private final String _extension; + }
\ No newline at end of file |