Performance enhancement in HttpSessionRequestCache

[shin-mallang]

savedRequest is only used in conditional statements.

Thus I moved the constructor for DefaultSavedRequest into the conditional statements.

Then it will be only executed when conditions met.

Accordingly, I expect that there are some performance improvements.

[sjohnr]

Thanks @ShinDongHun1. Do you have any way to quantify the performance improvement of this change? For example, how much faster is it for 1000 requests, etc.? I'm not sure we would want to push the change if we don't have a good answer for what the improvement/impact actually is, since this change would appear in the release notes.

[shin-mallang]

@sjohnr Thank you for answer.

I tested with 10000 requests after setting the createSessionAllowed attribute to false and 1.5 times faster on average. and 50000 request, 2 times faster on average.

But the difference is only a few tens of milliseconds.

It's a pity that my code won't be reflected with this performance improvement, but I don't think there's any reason not to apply it if there is a better alternative.

I don't think my code will be reflected because the performance improvement is so small. :(

But I don't think there's any reason not to include better code.

I'm sorry I'm not good at English, and thank you so much for your reply! it was a good experience :)

[sjohnr]

I'm sorry I'm not good at English, and thank you so much for your reply! it was a good experience :)

No problem, we can try our best to understand each other! I'm really glad you're enjoying contributing!

I tested with 10000 requests after setting the createSessionAllowed attribute to false and 1.5 times faster on average. and 50000 request, 2 times faster on average.

So are you saying that with 50000 requests, you saw a 2x speed increase per request, for example in 10 milliseconds per request instead of 20 milliseconds? If so, that is definitely worth pursuing!

I don't think my code will be reflected because the performance improvement is so small.

It doesn't sound small to me! But we'll need to make sure. I'll also see if I can find time to run a few tests and verify your findings. I'll reply back when I've had a chance to look at this deeper.

[shin-mallang]

So are you saying that with 50000 requests, you saw a 2x speed increase per request, for example in 10 milliseconds per request instead of 20 milliseconds?

umm.. I meant the total time that all requests are processed, not per request

Previously, 50000 requests were processed within 90ms in my computer(macbook m1 pro). But, after modification of the code I did, it was averagely done within 45ms.

I tested using 200 threads via Executors.newFixedThreadPool(200).

thank you for your reply :)

[sjohnr]

umm.. I meant the total time that all requests are processed, not per request

Ok. I think it works out about the same either way. It doesn't sound like a lot but 45 ms could be a lot.

[sjohnr]

@ShinDongHun1, thanks for your patience.

I put together a pretty simple test to measure performance differences with this code change. I tested just the method cache.saveRequest(request, response) with 100,000,000 iterations, and ran the test a number of times before and after the code change. The request that was saved included 4 headers, 4 cookies, 4 preferred locales and 4 parameters, causing the constructor to do some work on each iteration.

There were no discernible differences with the default configuration for HttpSessionRequestCache. However, with cache.setCreateSessionAllowed(false):

Before code change:

• Average ~1,464 nanoseconds per saveRequest

After code change:

• Average ~114 nanoseconds per saveRequest

So we save about 1.35 microseconds per request. This is pretty small, but proves that the performance impact is non-negligible. It's worth noting that most applications would use the NullRequestCache in the stateless scenario (sessionManagement().sessionCreationPolicy(SessionCreationPolicy.STATELESS) or even SessionCreationPolicy.NEVER), so I'm not sure many applications will benefit from this. Still, this is another way to configure the application to avoid session creation when desired and may improve performance in that case.

[sjohnr]

Merged into 5.8.x as 4ff0724c87ee306e23a3758480417024e762915e