Skip to content

Port Hibernate's EntityKey optimization #1972

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
Jan 14, 2019

Conversation

maca88
Copy link
Contributor

@maca88 maca88 commented Jan 13, 2019

This PR ports Hibernate's HHH-8682, that aims to have an EntityKey as smallest as possible. In addition the ManyToOneType.ScheduleBatchLoadIfNeeded was modified to not generate an entity key when batching is disabled in order to reduce garbage collection. Ideally, an object pool could be implemented in order to prevent generating keys just to check if the key exist in the persistence context or not as from profiling, 35 percent of the time running the "set fetches" test is spent by the GC.

Here are some benchmark results from RawDataAccessBencher, before:

Async eager Load fetches
-------------------------
# of elements fetched: 6768 (4768 + 1000 + 1000).    Fetch took: 90.673.920,00ms. Allocated bytes: 90673920.

Change tracking fetches, set fetches (20 runs), no caching
------------------------------------------------------------------------------
NHibernate v5.2.0.0 (v5.2.2.0) : 2.050,20ms (58,98ms)   Enum: 1,71ms (0,08ms)

Memory usage, per iteration
------------------------------------------------------------------------------
NHibernate v5.2.0.0 (v5.2.2.0) : 195.868 KB (200.569.192 bytes)

after:

Async eager Load fetches
-------------------------
# of elements fetched: 6768 (4768 + 1000 + 1000).    Fetch took: 87.064.984,00ms. Allocated bytes: 87064984.

Change tracking fetches, set fetches (20 runs), no caching
------------------------------------------------------------------------------
NHibernate v5.2.0.0 (v5.2.2.0) : 1.853,30ms (57,91ms)   Enum: 1,67ms (0,06ms)

Memory usage, per iteration
------------------------------------------------------------------------------
NHibernate v5.2.0.0 (v5.2.2.0) : 173.056 KB (177.209.960 bytes)

@maca88 maca88 changed the title Port's Hibernate EntityKey optimization Port Hibernate's EntityKey optimization Jan 13, 2019
@fredericDelaporte
Copy link
Member

For merging, I intend to squash and merge in order to reword the commit.

@hazzik
Copy link
Member

hazzik commented Jan 13, 2019

Need to implement IEquitable to squeeze more performance.

@maca88
Copy link
Contributor Author

maca88 commented Jan 13, 2019

Done.

@hazzik
Copy link
Member

hazzik commented Jan 13, 2019

Done

Can you run a test again?

@maca88
Copy link
Contributor Author

maca88 commented Jan 13, 2019

The results are more or less the same:

Async eager Load fetches
-------------------------
# of elements fetched: 6768 (4768 + 1000 + 1000).    Fetch took: 87.025.824,00ms. Allocated bytes: 87025824.

Results per framework. Values are given as: 'mean (standard deviation)'
==============================================================================

Change tracking fetches, set fetches (20 runs), no caching
------------------------------------------------------------------------------
NHibernate v5.2.0.0 (v5.2.2.0) : 1.851,49ms (51,74ms)   Enum: 1,72ms (0,11ms)

Memory usage, per iteration
------------------------------------------------------------------------------
NHibernate v5.2.0.0 (v5.2.2.0) : 173.071 KB (177.225.576 bytes)

@bahusoid
Copy link
Member

bahusoid commented Jan 14, 2019

I think converting EntityKey to struct should squeeze some more perf. I know it's a breaking change but it would be interesting to see test results

@bahusoid
Copy link
Member

bahusoid commented Jan 14, 2019

@maca88 I've prepared a patch for this branch with EntityKey class->struct conversion:
0001-Convert-EntityKey-to-struct.zip
It can be applied via git apply 0001-Convert-EntityKey-to-struct.patch for unzipped file placed in root nhibernate-core folder. Could you please check how it affects benchmark results.

@maca88
Copy link
Contributor Author

maca88 commented Jan 14, 2019

By just appling your patch the overall performance wasn't changed as the GenerateEntityKey is not called anymore:

Async eager Load fetches
-------------------------
# of elements fetched: 6768 (4768 + 1000 + 1000).    Fetch took: 86.105.704,00ms. Allocated bytes: 86105704.

Results per framework. Values are given as: 'mean (standard deviation)'
==============================================================================

Change tracking fetches, set fetches (20 runs), no caching
------------------------------------------------------------------------------
NHibernate v5.2.0.0 (v5.2.2.0) : 1.850,72ms (79,86ms)   Enum: 1,71ms (0,10ms)

Memory usage, per iteration
------------------------------------------------------------------------------
NHibernate v5.2.0.0 (v5.2.2.0) : 167.546 KB (171.567.952 bytes)

By reverting the change made in ManyToOneType then there is a difference:
EntityKey as class:
class

EntityKey as struct:
struct

We can see that struct has better performance in this case. The problem having EntityKey as struct is that its size can pass the 16 bytes that Microsoft recommends for a struct. In our case the size of EntityKey is 16 bytes as the identifier is an int and the test was ran on a x64 platform, but if the identifier would be a string then its size would be 20 20 bytes.
Another option for optimizing such cases is to use the object pooling that is frequently used in games. In our case we could store the pooled entity key on the session:

private EntityKey _entityKey;
public EntityKey GetPooledEntityKey(object id, IEntityPersister persister) {
  if (_entityKey == null)
    _entityKey = new EntityKey(id, persister);
  else
    _entityKey.Update(id, persister);
  return _entityKey;
}

and the result would be:
pooled

@bahusoid

This comment has been minimized.

@maca88

This comment has been minimized.

@bahusoid

This comment has been minimized.

@maca88

This comment has been minimized.

@hazzik
Copy link
Member

hazzik commented Jan 14, 2019

By just appling your patch the overall performance wasn't changed

But memory usage dropped by ~6 megabytes.

@hazzik hazzik merged commit 14c5cdb into nhibernate:master Jan 14, 2019
@fredericDelaporte

This comment has been minimized.

@maca88

This comment has been minimized.

@maca88 maca88 deleted the EntityKeyOptimization branch September 27, 2019 20:32
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.

4 participants