[security-dev] Picketbox performance improvements

[Peter Skopek]

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 at redhat.com>
>>> To: "Stuart Douglas" <sdouglas at redhat.com>
>>> Cc: "Anil Saldhana" <Anil.Saldhana at redhat.com>, security-dev at lists.jboss.org, "Andrig Miller" <anmiller at redhat.com>
>>> 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 at redhat.com>
>>>>> To: "Stuart Douglas" <sdouglas at redhat.com>
>>>>> Cc: security-dev at lists.jboss.org, "Andrig Miller" <anmiller at redhat.com>
>>>>> 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 at lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/security-dev
>