spring-session icon indicating copy to clipboard operation
spring-session copied to clipboard

Reduce allocations from Redis operations

Open theigl opened this issue 1 year ago • 2 comments

RedisIndexedSessionRepository and RedisSessionExpirationPolicy create a lot of single-use proxies by relying on BoundHashOperations.

For instance in this case:

https://github.com/spring-projects/spring-session/blob/f6f4c8652e03257420bc35f44492c5dc5d4121a9/spring-session-data-redis/src/main/java/org/springframework/session/data/redis/RedisIndexedSessionRepository.java#L898-L903

https://github.com/spring-projects/spring-session/blob/f6f4c8652e03257420bc35f44492c5dc5d4121a9/spring-session-data-redis/src/main/java/org/springframework/session/data/redis/RedisIndexedSessionRepository.java#L733-L735

This showed up in our production profiler because of hundreds of MB allocated in java.lang.reflect.Method:

image

If I understand the code correctly, this is done purely for convenience and could simply be replaced by the following code:

String key = getSessionKey(sessionId);
sessionRedisOperations.opsForHash().putAll(key, this.delta);

Or am I overlooking something?

theigl avatar Sep 19 '24 09:09 theigl

@marcusdacoregio: Hi and sorry for pinging you directly.

I'd be willing to provide a PR for this if I get a preliminary OK on my suggestion. I think the performance optimization would be worth it.

theigl avatar Oct 03 '24 11:10 theigl

Hi @theigl, thanks for reaching out.

Unfortunately, I'm not in the project anymore so I won't be able to approve/merge your PR if submitted. I'll ping @rwinch here who might be able to guide you.

I'd say that the best thing to do is to create a copy implementation of RedisIndexedSessionRepository, apply your changes and measure it again, that will help the team analyze your suggestion better.

marcusdacoregio avatar Oct 03 '24 17:10 marcusdacoregio

@rwinch: Would you accept a PR for this?

theigl avatar Apr 08 '25 08:04 theigl