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

Refactored mock cache to be non-blocking. #470

Merged
merged 1 commit into from Jun 28, 2016
Merged

Conversation

raphw
Copy link
Member

@raphw raphw commented Jun 26, 2016

The previous cache used a lock to unify concurrency and weak references. Instead, keys are now explicitly wrapped using weak references and similarly to the functioning of a weak hash map, the entries are polled upon accessing the cache. This avoids blocking.

@codecov-io
Copy link

codecov-io commented Jun 26, 2016

Current coverage is 87.64%

Sunburst

No coverage report found for master at 2f50871.

Powered by Codecov. Last updated by 2f50871...fabed34

generator = avoidingClassLeakageCache.putIfAbsent(new WeakKey<ClassLoader>(classLoader, this), newGenerator = new CachedBytecodeGenerator(mockBytecodeGenerator, weak));
if (generator == null) {
generator = newGenerator;
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd change that code to one variable declaration per line. And I would avoid inline assignment too (the first newGenerator is not needed before the if).
Those too may confuse most readers.

return (Class<T>) generatedMockClass;
} finally {
avoidingClassLeakCacheLock.unlock();
void pollAndClearReferences() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would like a comment on this, or maybe rename it better (I'm thinking loudly).
Not everyone knows ReferenceQueue or how it works, and thus why we need to invoke this method upon get(...).

Ideas welcome.

@bric3
Copy link
Contributor

bric3 commented Jun 27, 2016

Looks good codewise.
I wonder if coverage can be increased however.

@bric3
Copy link
Contributor

bric3 commented Jun 27, 2016

LGTM

@raphw raphw merged commit 3e0f316 into master Jun 28, 2016
@bric3 bric3 deleted the non-blocking-cache branch June 28, 2016 15:23
@bric3 bric3 added this to the 2.1 milestone Aug 30, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants