[security-dev] Picketbox performance improvements

[Stuart Douglas]

Does this include my performance fixes?

Stuart

----- Original Message -----
> From: "Peter Skopek" <pskopek at redhat.com>
> To: security-dev at 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 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
> > 
> _______________________________________________
> security-dev mailing list
> security-dev at lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/security-dev
>