Skip to content

Fix ProxyFactory cache #1454

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

Merged
merged 2 commits into from
Dec 5, 2017
Merged

Conversation

hazzik
Copy link
Member

@hazzik hazzik commented Nov 24, 2017

  • use a correct implementation of cache
  • reduce the number of allocations of ProxyCacheEntry
  • reduce memory flow

ProxyCache has a weird, inefficient and incorrect implementation. Replaced with just ConcurrentDictionary.
Removed unnecessary collections manipulations to get some perf boost.
Also, previously there were 4 instances of ProxyCacheEntry created per one call to CreateProxyType. But the constructor of ProxyCacheEntry is expensive in terms of both memory and CPU.

@hazzik hazzik changed the base branch from master to 5.0.x November 24, 2017 13:22
@hazzik hazzik added this to the 5.0.2 milestone Nov 24, 2017
@hazzik
Copy link
Member Author

hazzik commented Nov 25, 2017

Going to include into 5.1 instead. As #1451 now uses completely different approach, I’m going to remove from there changes related to this PR.

@hazzik hazzik modified the milestones: 5.0.2, 5.1 Nov 25, 2017
@hazzik hazzik closed this Nov 25, 2017
@hazzik hazzik reopened this Dec 2, 2017
@hazzik hazzik removed the r: Replaced label Dec 2, 2017
@hazzik hazzik changed the base branch from 5.0.x to master December 3, 2017 07:28
@hazzik hazzik force-pushed the entity-key-allocations branch 2 times, most recently from a57a49e to 41f16d0 Compare December 3, 2017 08:22
- use a correct implementation of cache
- reduce number of allocations of ProxyCacheEntry
- reduce memory flow
@hazzik hazzik force-pushed the entity-key-allocations branch from 41f16d0 to ee06c89 Compare December 3, 2017 08:34
using NHibernate.Proxy;
using NHibernate.Proxy.DynamicProxy;
using NUnit.Framework;

namespace NHibernate.Test.NHSpecificTest.NH3954
{
[TestFixture, Explicit("Demonstrates bug impact on cache, but which tests will fail is a bit unpredictable")]
[TestFixture, Explicit("Demonstrates bug impact on cache, but which tests will fail is a bit unpredictable"), Obsolete]
Copy link
Member Author

Choose a reason for hiding this comment

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

Had to mark it as obsolete as ProxyCache class now is marked obsolete

private int _hashCode1;
private int _hashCode2;

private static readonly FieldInfo InternalCacheField =
typeof(ProxyCache).GetField("cache", BindingFlags.Static | BindingFlags.NonPublic);
typeof(ProxyFactory).GetField("_cache", BindingFlags.Static | BindingFlags.NonPublic);
Copy link
Member Author

Choose a reason for hiding this comment

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

The field has moved to ProxyFactory

@hazzik hazzik merged commit 970e9fa into nhibernate:master Dec 5, 2017
@hazzik hazzik deleted the entity-key-allocations branch December 5, 2017 20:48
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.

2 participants