1:25 a.m.
Hi
I’m aware this may no technically be the right list to discuss this but
this list is impacted by this and fairly active.
During load testing of our application we found a case we spend 10% of
your CPU time in SimpleRole#equals (see attachment). This is because
SimpleRoleGroup uses an ArrayList to maintain a unique set of roles. As
a result it has to call ArrayList#contains a lot, which is itself O(n).
In fact because that’s done when iterating over all the roles it becomes
O(n^2). In our case our principals can have up to 200 roles. I don’t
know if this is exceptionally many or a common case.
We would like to work on a patch but we need your guidance. There are
several possible solutions and it all comes down to whether we can
change the List in the RoleGroup interface to a Collection. All the
users we searched for in WildFly only used the return value for
iterating over, nobody used the index. In fact they all used it an a
for-each loop so the change would even be source compatible but
unfortunately not binary compatible. If we can change the interface then
we can just change the ArrayList in SimpleRoleGroup to a HashSet and be
done with it. If the order is important the we can either use a TreeSet
or a LinkedHashSet.
If changing the RoleGroup interface is not possible then there are two
other possibilities. The first is only internally using a Set but in
getRoles perform a copy. This would produce more garbage. The second
option would be a having a Set and List (to avoid having to do copies)
in SimpleRoleGroup. This would avoid having to do a copy and the Set can
be used for contains checks. Only removeRole would still be O(n) due to
the scan over ArrayList. The only user we found was
OptionsRoleMappingProvider.
All of these would change the serialized form. If it is important to
support reading old instances that could be added as well.
Cheers
Philippe
5:08 a.m.
Hi,
On Mon, Jun 8, 2015 at 8:25 AM, Philippe Marschall <kustos(a)gmx.net> wrote:
Hi
I’m aware this may no technically be the right list to discuss this but
this list is impacted by this and fairly active.
During load testing of our application we found a case we spend 10% of
your CPU time in SimpleRole#equals (see attachment). This is because
SimpleRoleGroup uses an ArrayList to maintain a unique set of roles. As a
result it has to call ArrayList#contains a lot, which is itself O(n). In
fact because that’s done when iterating over all the roles it becomes
O(n^2). In our case our principals can have up to 200 roles. I don’t know
if this is exceptionally many or a common case.
200 doesn't seem like so many. The "problem" often is that the term
"role"
makes people think they are only allowed to use it for things like
"administrator" and "manager" and such. In that view 200 may be much.
But, nothing in Java EE security imposes this, and if you want to use roles
with names like "can_see_updated_interest", then 200 is really nothing.
From the call stack, I guess this is triggered via @RolesAllowed on an
EJB
right?
Theoretically, this is where JACC could come in according to the Java EE
standards. This would allow you to implement your own (optimized if
necessary) logic for "is user/caller in role".
However despite being Java EE 7 certified, it seems it's not actually
possible to install a JACC provider on JBoss. This is a bit of a spec hole,
unfortunately. Most of the code seems to be there in JBoss (WildFly), but
there's just no place where you can actually put your own JACC provider.
Kind regards,
Arjan Tijms
8:20 a.m.
The order of the roles is not important. I think we should change the
SimpleRole implementation in PicketBox to use Collection (HashSet)
reference instead of List and then change any WildFly code that might be
using the List reference.
On 06/08/2015 03:25 AM, Philippe Marschall wrote:
Hi
I’m aware this may no technically be the right list to discuss this
but this list is impacted by this and fairly active.
During load testing of our application we found a case we spend 10% of
your CPU time in SimpleRole#equals (see attachment). This is because
SimpleRoleGroup uses an ArrayList to maintain a unique set of roles.
As a result it has to call ArrayList#contains a lot, which is itself
O(n). In fact because that’s done when iterating over all the roles it
becomes O(n^2). In our case our principals can have up to 200 roles. I
don’t know if this is exceptionally many or a common case.
We would like to work on a patch but we need your guidance. There are
several possible solutions and it all comes down to whether we can
change the List in the RoleGroup interface to a Collection. All the
users we searched for in WildFly only used the return value for
iterating over, nobody used the index. In fact they all used it an a
for-each loop so the change would even be source compatible but
unfortunately not binary compatible. If we can change the interface
then we can just change the ArrayList in SimpleRoleGroup to a HashSet
and be done with it. If the order is important the we can either use a
TreeSet or a LinkedHashSet.
If changing the RoleGroup interface is not possible then there are two
other possibilities. The first is only internally using a Set but in
getRoles perform a copy. This would produce more garbage. The second
option would be a having a Set and List (to avoid having to do copies)
in SimpleRoleGroup. This would avoid having to do a copy and the Set
can be used for contains checks. Only removeRole would still be O(n)
due to the scan over ArrayList. The only user we found was
OptionsRoleMappingProvider.
All of these would change the serialized form. If it is important to
support reading old instances that could be added as well.
Cheers
Philippe
_______________________________________________
wildfly-dev mailing list
wildfly-dev(a)lists.jboss.org
https://lists.jboss.org/mailman/listinfo/wildfly-dev
8:42 a.m.
On 06/08/2015 07:08 AM, arjan tijms wrote:
Theoretically, this is where JACC could come in according to the Java
EE standards. This would allow you to implement your own (optimized if
necessary) logic for "is user/caller in role".
However despite being Java EE 7 certified, it seems it's not actually
possible to install a JACC provider on JBoss. This is a bit of a spec
hole, unfortunately. Most of the code seems to be there in JBoss
(WildFly), but there's just no place where you can actually put your
own JACC provider.
Huh? We would not be certified if we didn't support custom providers as
the TCK requires the installation of its own provider. The
javax.security.jacc.PolicyConfigurationFactory.provider and
javax.security.jacc.policy.provider system properties are both supported
by WildFly.
Kind regards,
Arjan Tijms
_______________________________________________
wildfly-dev mailing list
wildfly-dev(a)lists.jboss.org
https://lists.jboss.org/mailman/listinfo/wildfly-dev
8:55 a.m.
Hi,
On Mon, Jun 8, 2015 at 3:42 PM, Stefan Guilhen <sguilhen(a)redhat.com> wrote:
On 06/08/2015 07:08 AM, arjan tijms wrote:
Theoretically, this is where JACC could come in according to the Java EE
standards. This would allow you to implement your own (optimized if
necessary) logic for "is user/caller in role".
However despite being Java EE 7 certified, it seems it's not actually
possible to install a JACC provider on JBoss. This is a bit of a spec hole,
unfortunately. Most of the code seems to be there in JBoss (WildFly), but
there's just no place where you can actually put your own JACC provider.
Huh? We would not be certified if we didn't support custom providers as
the TCK requires the installation of its own provider. The
javax.security.jacc.PolicyConfigurationFactory.provider and
javax.security.jacc.policy.provider system properties are both supported by
WildFly.
Indeed, you would say that. But I do know that a TCK can be lacking "a
little", specifically where it concerns JASPIC and JACC, unfortunately.
There are a few other major violations in various certified products. With
JASPIC for instance it was possible to certify a product that just didn't
implement the actual authentication, which is the core of the core of
JASPIC :X
Those properties are of course supported, but where does one put the
classes (or jar containing these classes)? I tried for hours at end and
asked in the JBoss forum, but it never became clear. The documentation
doesn't mention it either. See this for my question about this:
https://developer.jboss.org/thread/254106
Would be really cool if the location could become clear. Thanks!
Kind regards,
Arjan Tijms
Kind regards,
Arjan Tijms
_______________________________________________
wildfly-dev mailing
listwildfly-dev@lists.jboss.orghttps://lists.jboss.org/mailman/listinfo/wildfly-dev
_______________________________________________
wildfly-dev mailing list
wildfly-dev(a)lists.jboss.org
https://lists.jboss.org/mailman/listinfo/wildfly-dev
1:26 p.m.
Arjan,
On 06/08/2015 10:55 AM, arjan tijms wrote:
Hi,
Indeed, you would say that. But I do know that a TCK can be lacking "a
little", specifically where it concerns JASPIC and JACC,
unfortunately. There are a few other major violations in various
certified products. With JASPIC for instance it was possible to
certify a product that just didn't implement the actual
authentication, which is the core of the core of JASPIC :X
The TCK coverage (or lack thereof) has nothing to do with the ability to
configure custom JACC providers. If we didn't support it not a single
test of the JACC testsuite would pass so the TCK is not to be blamed in
this case. The JASPIC testsuite is another story and I think we both
agree that it is broken from our previous conversations. Arun's JEE
testsuite, to which you contributed your JASPIC tests, has been much
more valuable as a tool to validate the implementation than the TCK itself.
Having said that, the documentation does really seem to be missing a
section about custom JACC providers so I went to check the TCK setup. It
looks like the TCK JACC providers are bundled in a jar and this jar is
being set as a resource of the org.jboss.as.security module. I'm not
sure why it was done this way but I believe it should be also possible
to define your own module containing the classes and then wire it to the
security module as a dependency instead of a resource.
Those properties are of course supported, but where does one put the
classes (or jar containing these classes)? I tried for hours at end
and asked in the JBoss forum, but it never became clear. The
documentation doesn't mention it either. See this for my question
about this: https://developer.jboss.org/thread/254106
Would be really cool if the location could become clear. Thanks!
Kind regards,
Arjan Tijms
>
> Kind regards,
> Arjan Tijms
>
>
> _______________________________________________
> wildfly-dev mailing list
> wildfly-dev(a)lists.jboss.org <mailto:wildfly-dev@lists.jboss.org>
> https://lists.jboss.org/mailman/listinfo/wildfly-dev
_______________________________________________
wildfly-dev mailing list
wildfly-dev(a)lists.jboss.org <mailto:wildfly-dev@lists.jboss.org>
https://lists.jboss.org/mailman/listinfo/wildfly-dev
2:04 p.m.
Hi there,
On Mon, Jun 8, 2015 at 8:26 PM, Stefan Guilhen <sguilhen(a)redhat.com> wrote:
The TCK coverage (or lack thereof) has nothing to do with the
ability to
configure custom JACC providers. If we didn't support it not a single test
of the JACC testsuite would pass so the TCK is not to be blamed in this
case.
Well, one of the requirements of JACC is that the Java EE product has a
default JACC provider (which implements the authorization algorithms as
defined by both the Servlet and EJB containers). So it could
*theoretically* have been the case that the TCK only tests that one. Of
course I don't know if this is indeed the case.
JBoss indeed has such a default provider. However, the spec also requires
(I think) that in a full Java EE server all authorization decisions (for
the APIs defined by Servlet and EJB) go via JACC, which clearly does not
happen in JBoss. As far as I know, only GlassFish really does this, while
TMaxSoft JEUS comes close.
The JASPIC testsuite is another story and I think we both agree that
it is
broken from our previous conversations. Arun's JEE testsuite, to which you
contributed your JASPIC tests, has been much more valuable as a tool to
validate the implementation than the TCK itself.
I'm really glad they helped ;)
Having said that, the documentation does really seem to be missing a
section about custom JACC providers so I went to check the TCK setup. It
looks like the TCK JACC providers are bundled in a jar and this jar is
being set as a resource of the org.jboss.as.security module. I'm not sure
why it was done this way but I believe it should be also possible to define
your own module containing the classes and then wire it to the security
module as a dependency instead of a resource.
Hmmm, how would one go about doing that exactly? I think I created a module
for my custom JACC provider, then set that as a dependency for the security
module (since that was the place the default implementation lives), but it
again did not work (class not found exceptions). Could well be the case
that I did something wrong, so an example would be great.
Kind regards,
Arjan Tijms
Those properties are of course supported, but where does one put the
classes (or jar containing these classes)? I tried for hours at end and
asked in the JBoss forum, but it never became clear. The documentation
doesn't mention it either. See this for my question about this:
https://developer.jboss.org/thread/254106
Would be really cool if the location could become clear. Thanks!
Kind regards,
Arjan Tijms
>
>
> Kind regards,
> Arjan Tijms
>
>
> _______________________________________________
> wildfly-dev mailing
listwildfly-dev@lists.jboss.orghttps://lists.jboss.org/mailman/listinfo/wildfly-dev
>
>
>
> _______________________________________________
> wildfly-dev mailing list
> wildfly-dev(a)lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/wildfly-dev
>
12:37 a.m.
Very well, we can do that. How do you feel about breaking the serialized
form?
On 08.06.2015 15:20, Stefan Guilhen wrote:
The order of the roles is not important. I think we should change
the
SimpleRole implementation in PicketBox to use Collection (HashSet)
reference instead of List and then change any WildFly code that might be
using the List reference.
On 06/08/2015 03:25 AM, Philippe Marschall wrote:
> Hi
>
> I’m aware this may no technically be the right list to discuss this
> but this list is impacted by this and fairly active.
>
> During load testing of our application we found a case we spend 10% of
> your CPU time in SimpleRole#equals (see attachment). This is because
> SimpleRoleGroup uses an ArrayList to maintain a unique set of roles.
> As a result it has to call ArrayList#contains a lot, which is itself
> O(n). In fact because that’s done when iterating over all the roles it
> becomes O(n^2). In our case our principals can have up to 200 roles. I
> don’t know if this is exceptionally many or a common case.
>
> We would like to work on a patch but we need your guidance. There are
> several possible solutions and it all comes down to whether we can
> change the List in the RoleGroup interface to a Collection. All the
> users we searched for in WildFly only used the return value for
> iterating over, nobody used the index. In fact they all used it an a
> for-each loop so the change would even be source compatible but
> unfortunately not binary compatible. If we can change the interface
> then we can just change the ArrayList in SimpleRoleGroup to a HashSet
> and be done with it. If the order is important the we can either use a
> TreeSet or a LinkedHashSet.
> If changing the RoleGroup interface is not possible then there are two
> other possibilities. The first is only internally using a Set but in
> getRoles perform a copy. This would produce more garbage. The second
> option would be a having a Set and List (to avoid having to do copies)
> in SimpleRoleGroup. This would avoid having to do a copy and the Set
> can be used for contains checks. Only removeRole would still be O(n)
> due to the scan over ArrayList. The only user we found was
> OptionsRoleMappingProvider.
> All of these would change the serialized form. If it is important to
> support reading old instances that could be added as well.
>
> Cheers
> Philippe
>
>
> _______________________________________________
> wildfly-dev mailing list
> wildfly-dev(a)lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/wildfly-dev
_______________________________________________
wildfly-dev mailing list
wildfly-dev(a)lists.jboss.org
https://lists.jboss.org/mailman/listinfo/wildfly-dev
7:51 a.m.
You don't have to break the serialized form to make this kind of change.
Have a look at this:
https://docs.oracle.com/javase/7/docs/platform/serialization/spec/serial-...
Set that up, plus a customized readObject()/writeObject(), and you can
retain serialization compatibility.
On 06/09/2015 12:37 AM, Philippe Marschall wrote:
Very well, we can do that. How do you feel about breaking the
serialized
form?
On 08.06.2015 15:20, Stefan Guilhen wrote:
> The order of the roles is not important. I think we should change the
> SimpleRole implementation in PicketBox to use Collection (HashSet)
> reference instead of List and then change any WildFly code that might be
> using the List reference.
>
> On 06/08/2015 03:25 AM, Philippe Marschall wrote:
>> Hi
>>
>> I’m aware this may no technically be the right list to discuss this
>> but this list is impacted by this and fairly active.
>>
>> During load testing of our application we found a case we spend 10% of
>> your CPU time in SimpleRole#equals (see attachment). This is because
>> SimpleRoleGroup uses an ArrayList to maintain a unique set of roles.
>> As a result it has to call ArrayList#contains a lot, which is itself
>> O(n). In fact because that’s done when iterating over all the roles it
>> becomes O(n^2). In our case our principals can have up to 200 roles. I
>> don’t know if this is exceptionally many or a common case.
>>
>> We would like to work on a patch but we need your guidance. There are
>> several possible solutions and it all comes down to whether we can
>> change the List in the RoleGroup interface to a Collection. All the
>> users we searched for in WildFly only used the return value for
>> iterating over, nobody used the index. In fact they all used it an a
>> for-each loop so the change would even be source compatible but
>> unfortunately not binary compatible. If we can change the interface
>> then we can just change the ArrayList in SimpleRoleGroup to a HashSet
>> and be done with it. If the order is important the we can either use a
>> TreeSet or a LinkedHashSet.
>> If changing the RoleGroup interface is not possible then there are two
>> other possibilities. The first is only internally using a Set but in
>> getRoles perform a copy. This would produce more garbage. The second
>> option would be a having a Set and List (to avoid having to do copies)
>> in SimpleRoleGroup. This would avoid having to do a copy and the Set
>> can be used for contains checks. Only removeRole would still be O(n)
>> due to the scan over ArrayList. The only user we found was
>> OptionsRoleMappingProvider.
>> All of these would change the serialized form. If it is important to
>> support reading old instances that could be added as well.
>>
>> Cheers
>> Philippe
>>
>>
>> _______________________________________________
>> wildfly-dev mailing list
>> wildfly-dev(a)lists.jboss.org
>> https://lists.jboss.org/mailman/listinfo/wildfly-dev
>
>
>
>
> _______________________________________________
> wildfly-dev mailing list
> wildfly-dev(a)lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/wildfly-dev
>
_______________________________________________
wildfly-dev mailing list
wildfly-dev(a)lists.jboss.org
https://lists.jboss.org/mailman/listinfo/wildfly-dev
--
- DML
10:37 a.m.
Hi,
On Mon, Jun 8, 2015 at 9:04 PM, arjan tijms <arjan.tijms(a)gmail.com> wrote:
Having said that, the documentation does really seem to be missing a
> section about custom JACC providers so I went to check the TCK setup. It
> looks like the TCK JACC providers are bundled in a jar and this jar is
> being set as a resource of the org.jboss.as.security module. I'm not sure
> why it was done this way but I believe it should be also possible to define
> your own module containing the classes and then wire it to the security
> module as a dependency instead of a resource.
>
Hmmm, how would one go about doing that exactly? I think I created a
module for my custom JACC provider, then set that as a dependency for the
security module (since that was the place the default implementation
lives), but it again did not work (class not found exceptions). Could well
be the case that I did something wrong, so an example would be great.
Sorry to ask again, but still wondering how to set a custom JACC provider ;)
Incidentally, I found a rather grave bug in the default JACC provider where
it checks principals purely based on their name, without taking their type
into account. See https://issues.jboss.org/browse/WFLY-5740
Kind regards,
Arjan Tijms
3074
days inactive
3245
days old
9 comments
4 participants
participants (4)
-
arjan tijms -
David M. Lloyd -
Philippe Marschall -
Stefan Guilhen