Dev/kennyy/bigger event cache #399
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Caches lookups from the Events table, since we make a lot of calls to this. This eliminates 5 server roundtrips per page load. The cache expires after 1 minute and is shared among all users and threads.
Caching the results of is player/author/admin, as well as the lookup of PuzzleUsers. Expiry is left at 1 minute so we can make changes to the event and have them take effect rapidly. These changes, plus the event cache itself, knocked 12-15 SQL requests off of the Puzzles page.
Closed
Vroo
reviewed
May 1, 2019
| @@ -11,6 +13,11 @@ namespace ServerCore.Helpers | |||
| /// </summary> | |||
| public static class EventHelper | |||
| { | |||
| private static ConcurrentDictionary<int, Event> eventCacheByInt = new ConcurrentDictionary<int, Event>(); | |||
| private static ConcurrentDictionary<string, Event> eventCacheByString = new ConcurrentDictionary<string, Event>(); | |||
| private static DateTime expiryTime = DateTime.MinValue; | |||
There was a problem hiding this comment.
I think you want volatile here.
| private static ConcurrentDictionary<int, Event> eventCacheByInt = new ConcurrentDictionary<int, Event>(); | ||
| private static ConcurrentDictionary<string, Event> eventCacheByString = new ConcurrentDictionary<string, Event>(); | ||
| private static DateTime expiryTime = DateTime.MinValue; | ||
| private static TimeSpan expiryWindow = TimeSpan.FromMinutes(1); |
| } | ||
| else | ||
| { | ||
| if (eventCacheByString.TryGetValue(eventId, out result) || (Int32.TryParse(eventId, out int eventIdInt) && eventCacheByInt.TryGetValue(eventIdInt, out result))) |
There was a problem hiding this comment.
If I'm understanding this correctly, you're caching the Event object, not the full results. Are all the puzzles cached indirectly by being instantiated as we enumerate them off of the event object? The caching I was thinking of doing (and I can still work on it if you'd like) is to cache the computed standings.
| return result; | ||
| } | ||
| } | ||
|
|
||
| // first, lookup by UrlString - this is the friendly name | ||
| result = await puzzleServerContext.Events.Where(e => e.UrlString == eventId).FirstOrDefaultAsync(); |
There was a problem hiding this comment.
You could combine these two queries into one:
Int32.TryParse(eventId, out int eventIdAsInt); // if 0 is a valid eventId, then need an if here
result = await puzzleServerContext.Events.Where(
e => e.UrlString == eventId || e.ID == eventIdAsInt)
.FirstOrDefaultAsync();
|
Abandoning; if we cache we should do it right. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.