Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed caching #153

Merged
merged 2 commits into from
May 30, 2020
Merged

Fixed caching #153

merged 2 commits into from
May 30, 2020

Conversation

Thorium
Copy link
Member

@Thorium Thorium commented May 29, 2020

There are 2 issues with the current caching. (Yes, I know it's taken from FSharp.Data.)

  1. The cache invalidation is not very good, but as we know it's endless rabbit hole, I'm not fixing it right now.
  2. The ConcurrentDictionary is used as normal dictionary, losing the benefit of it being ConcurrentDictionary. That is fixed with this PR. For more details, see: http://www.fssnip.net/7Qr/title/Correct-and-Incorrect-Way-to-Use-ConcurrentDictionary

@Thorium
Copy link
Member Author

Thorium commented May 29, 2020

This might or might not fix the issue of #150

@Thorium
Copy link
Member Author

Thorium commented May 30, 2020

What's wrong with macOS CI ? It says CI failed, but don't give any details.

@sergey-tihon sergey-tihon merged commit bbc6aa6 into fsprojects:master May 30, 2020
@sergey-tihon
Copy link
Member

Thank you @Thorium !
This fix released in v0.10.3

@Thorium
Copy link
Member Author

Thorium commented May 31, 2020

I think this is not enough. This is interesting article:
https://andrewlock.net/making-getoradd-on-concurrentdictionary-thread-safe-using-lazy/

Now, the question is: is the adding types so resource consuming operation, that it would be better to lock other threads meanwhile?

@Thorium
Copy link
Member Author

Thorium commented May 31, 2020

Added new PR #155 for that.

@Thorium Thorium deleted the cache-fix branch June 7, 2020 09:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants