Re: [security-dev] Picketbox performance improvements

From: "Peter Skopek" <pskopek(a)redhat.com&gt; To: security-dev(a)lists.jboss.org Sent: Wednesday, 11 September, 2013 12:15:34 PM Subject: Re: [security-dev] Picketbox performance improvements Hi Stuart, there is a PR https://github.com/wildfly/wildfly/pull/5029 to upgrade to new PicketBox which contains some fixes from Stefan in this matter. Peter On 09/05/2013 05:03 PM, Anil Saldhana wrote: > Hi Stuart, > Stefan and I started discussing the module organization for PicketBox > few days back. I think it is time for a separate email thread on that, > in this list. I think we are going to try to stick to "one jar per JBoss > module" mantra in WF8. > > Regards, > Anil > > On 09/05/2013 09:35 AM, Stuart Douglas wrote: >> I thought of that, however the default security context classes are not >> available from the maven module that houses the factory. Given that they >> all just get assembled into 1 jar anyway I wonder if they should just be >> merged into a single module? >> >> Stuart >> >> ----- Original Message ----- >>> From: "Stefan Guilhen" <sguilhen(a)redhat.com&gt; >>> To: "Stuart Douglas" <sdouglas(a)redhat.com&gt; >>> Cc: "Anil Saldhana" <Anil.Saldhana(a)redhat.com&gt;, >>> security-dev(a)lists.jboss.org, "Andrig Miller" <anmiller(a)redhat.com&gt; >>> Sent: Thursday, 5 September, 2013 3:47:27 PM >>> Subject: Re: [security-dev] Picketbox performance improvements >>> >>> Hi Stuart, >>> >>> I've applied your changes and they will be available once we upgrade >>> PicketBox, which should happen soon. I do believe that this heavy usage >>> of reflection is overkill since most of the time the default >>> SecurityContext implementation is used. I've never heard of a case where >>> this implemenation has been changed. So I'm wondering if we couldn't >>> just make sure that whenever the default context is being used we >>> instantiate it by calling new JBossSecurityContext(securityDomain) and >>> use the reflection stuff only if the caller really supplies a different >>> implementation class. >>> >>> Stefan >>> >>> On 09/05/2013 06:41 AM, Stuart Douglas wrote: >>>> Oops, just realised I missed that the same thing was happening with the >>>> SecurityContextUtil class. >>>> >>>> With this patch, and my recently merged SecurityContextAssociationValve >>>> patch, I have seen a >20% increase in performance of an empty web >>>> application (28k req/sec vs 23k req/sec). >>>> >>>> Stuart >>>> >>>> >>>> ----- Original Message ----- >>>>> From: "Anil Saldhana" <Anil.Saldhana(a)redhat.com&gt; >>>>> To: "Stuart Douglas" <sdouglas(a)redhat.com&gt; >>>>> Cc: security-dev(a)lists.jboss.org, "Andrig Miller" <anmiller(a)redhat.com&gt; >>>>> Sent: Wednesday, 4 September, 2013 3:47:47 PM >>>>> Subject: Re: [security-dev] Picketbox performance improvements >>>>> >>>>> Hi Stuart, >>>>> it will be a couple of days to upgrade. There are other fixes >>>>> going >>>>> into the upgrade. >>>>> >>>>> Regards, >>>>> Anil >>>>> >>>>> On 09/04/2013 04:31 AM, Stuart Douglas wrote: >>>>>> Hi, >>>>>> >>>>>> I have been benchmarking Wildfly upstream, and with the exception of >>>>>> the >>>>>> Weld listener (that I am about to try and fix), I am seeing the >>>>>> creation >>>>>> of the security context as by far the most expensive part of the web >>>>>> request chain. >>>>>> >>>>>> Firstly a bit about the tests, basically it is possible to run >>>>>> Undertow >>>>>> in >>>>>> pipelining mode, where if you send it pipelined HTTP requests it will >>>>>> buffer the responses and batch them. This is not super useful in >>>>>> practice, >>>>>> but what it does allow me to do is basically take most of the IO >>>>>> component >>>>>> out of a web request, and just look at which bits of the web request >>>>>> chain >>>>>> are consuming the most CPU. >>>>>> >>>>>> The issues I am seeing are: >>>>>> >>>>>> - Some unnecessary AccessController.doPrivilidged calls >>>>>> - A reflection call on every request to lookup the constructor of the >>>>>> security context class >>>>>> >>>>>> I have provided a patch to address both of these, by adding a guard >>>>>> around >>>>>> the AccessController calls, and by caching the constructor used for >>>>>> the >>>>>> default security context class. I think it may even be worthwhile >>>>>> taking >>>>>> this one step further, and using generated bytecode to create the >>>>>> class. >>>>>> Normally I would consider this overkill but security context creation >>>>>> happens on every request, so if we are serious about performance we >>>>>> should >>>>>> be trying to make it as cheap as possible. >>>>>> >>>>>> To give you an idea of how much this affects the total cost of the >>>>>> Servlet >>>>>> pipeline: >>>>>> >>>>>> Current WF upstream (without Weld): 134k req/sec >>>>>> After removing the AccessController calls: 158k req/sec >>>>>> Adding constructor caching: 171k req/sec >>>>>> >>>>>> Note that the speedup will not be as big for more realistic workloads, >>>>>> however it will still be significant. >>>>>> >>>>>> Stuart >>>>>> >>>>> > _______________________________________________ > security-dev mailing list > security-dev(a)lists.jboss.org > https://lists.jboss.org/mailman/listinfo/security-dev > _______________________________________________ security-dev mailing list security-dev(a)lists.jboss.org https://lists.jboss.org/mailman/listinfo/security-dev