Re: [security-dev] Picketbox performance improvements

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 >>>> >>>