Version 4.2.8

The puzzling bits

I've been looking at memory usage in YourKit as we're looking to reduce the memory footprint of our application, and I've come across a few puzzles which may be an opportunity to improve things in Spring Security.

DelegatingMethodSecurityMetadataSource.attributeCache has over 26000 objects.

  1. Within this there are references to Object.notify and Object.wait, as well as many internal Spring classes, which then map to empty values.

Could these entries be reclaimed such that attributeCache.get(key) returns null instead of returning an empty list?

  1. In terms of our security requirements, 99% of those 26000 entries will be junk. We should only have a relatively small number of @Controller and @Service methods which require security.

Could Spring Security emit a warning at end of scanning if their configuration is causing too much to be considered?

  1. Lastly, if I look at poor hash distribution, I find that DelegatingMethodSecurityMetadataSource.DefaultCacheKey is highlighted here.

Some of the hash collisions (I have up to 27 nodes for a table entry against an average of 1.3) are due to Method.hashCode() working with the method name and not the signature. I'm not sure that there's much that can be done here that would benefit as we cannot get at Method.signature which would give us a unique String being hashed.

Other hash collisions are more puzzling in that Java's Class.hashCode() seems to have hash collisions (scratching my head on this one).

My further analysis

I've edited this, as I've had a time to look further.

Regarding 1, it looks like emptyList() is used to indicate that the metadata is cached, so a non-existent entry is not possible.

It may be possible to globally blacklist (or more likely whitelist) all the methods from Object that are effectively just a side effect of all objects being derived from Object. I suspect that half of the entries in the attribute cache are from java.lang.Object.

Comment From: rwinch

Thanks for the feedback. Can you highlight what the actual problem you are trying to solve is? For example, is Spring Security's method security performing slowly? If so how do you reproduce?

Comment From: nealeu

Hi Rob.

We're trying to manage hosting costs by avoiding having to increase memory, so that's what had me look at what large objects are being retained. Anything that can be done to cut this down would be good.

I think that there is potential to improve memory, startup time and runtime performance by improving this area.

What do you think to having an option to whitelist the methods on Object by returning emptyList() if method.getDeclaringClass() is Object.class? [A local hack of this reduces the size from 26k entries to 20k].

This feels like it's just me seeing a special case opportunity to hack some memory back that doesn't belong in a framework.

Being able to override a default strategy however, would give the same benefit or more. When I filtered candidate classes to just those in the base package of our project, this dropped things down to 2k entries.

While I've not benchmarked this, I can speculate on what's going on with our project. The table underlying the 26k entries hashmap is sized at 32k (and therefore at least 128k bytes). The methods that we are interested in are distributed across that hashtable. The CPU is therefore being asked to frequently use large parts of something that is comparable to the size of the L2 cache.

Naturally, a micro-benchmark would be useful to demonstrate, however, as I said, it's the memory footprint that most interests me.

Comment From: rwinch

I don't think we can ignore methods on Object as this could potentially break existing applications. I think the ability to customize the cache seems like the best option as it provides more flexibility and does not introduce the potential risk of removing security from existing users.

To do this I think we should create two new classes

  • The first does what DelegatingMethodSecurityMetadataSource does but without caching within it
  • The second supports caching but delegates to a single MethodSecurityMetadataSource

Then DelegatingMethodSecurityMetadataSource would begin to delegate to these two implementations.

User's who want to provide their own caching would create their own implementation of the second class above.

Comment From: nealeu

Hi @rwinch,

That sounds a pretty good plan to me.

Cheers, Neale

Comment From: rwinch

@nealeu Any chance you would be interested in submitting a PR with the changes?

Comment From: nealeu

Hi Rob,

I'll have a look and see what I can do. I've been swamped, but may be able to do something now.

Comment From: nealeu

@rwinch Started on code for a PR. Aiming to get something within next few days around other commitments.

Comment From: nealeu

@rwinch I think we need a clear picture for GlobalMethodSecurityConfiguration which at the moment offers limited customisation via customMethodSecurityMetadataSource(), which is still then passed in to the default DelegatingMethodSecurityMetadataSource.

I think this is the way to go instead, and this method extraction could be cherry-picked to 4.x too for those who might be stuck in the past for some reason.

    @Bean
    public MethodSecurityMetadataSource methodSecurityMetadataSource() {
        List<MethodSecurityMetadataSource> sources = defaultMethodSecurityMetadataSources();
        return new DelegatingMethodSecurityMetadataSource(sources);
    }

From here it's now really easy to drop in our own implementation of DelegatingMethodSecurityMetadataSource().

In fact. I'm going to PR that commit as it's sufficient to make life much easier for everything else.

Comment From: spring-projects-issues

If you would like us to look at this issue, please provide the requested information. If the information is not provided within the next 7 days this issue will be closed.

Comment From: nealeu

@rwinch I'd be tempted to close this as per the automated reminder if I wasn't finally now doing the Spring MVC 4.x to Spring Boot 2.x shift on this codebase.

I'll have a check on the hash collision issue too once I've got things running in Spring Boot and see what I can do that paves an easy path such as an opt-in whitelist

Comment From: rwinch

Thanks. I'll mark this as waiting for feedback.

Comment From: spring-projects-issues

If you would like us to look at this issue, please provide the requested information. If the information is not provided within the next 7 days this issue will be closed.

Comment From: spring-projects-issues

Closing due to lack of requested feedback. If you would like us to look at this issue, please provide the requested information and we will re-open the issue.