[jira] [Commented] (LUCENENET-640) Sequential IndexWriter performance in concurrent environments.

["Shad Storhaug (Jira)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/LUCENENET-640?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17015015#comment-17015015 ] 

Shad Storhaug commented on LUCENENET-640:
-----------------------------------------

Hi Mathais,

Thanks for the report and the PR.

Correct me if I am wrong, but wouldn't a better fix for this to be to replace [{{WeakIdentityMap}}|https://github.com/apache/lucenenet/blob/master/src/Lucene.Net/Util/WeakIdentityMap.cs] with the thread-safe [{{ConditionalWeakTable}}|https://docs.microsoft.com/en-us/dotnet/api/system.runtime.compilerservices.conditionalweaktable-2]? We may still need to utilize {{IdentityWeakReference}}, but it would need to be a class since {{ConditionalWeakTable}} has a class constraint on {{TKey}}.

Do note that Microsoft didn't expose the enumerator or the [AddOrUpdate|https://docs.microsoft.com/en-us/dotnet/api/system.runtime.compilerservices.conditionalweaktable-2.addorupdate] method of {{ConditionalWeakTable}} until .NET Standard 2.1. However, Lucene requires one or the other in every (other) place where {{ConditionalWeakTable}} would be useful (specifically, as a replacement for [{{Lucene.Net.Support.WeakDictionary}}|https://github.com/apache/lucenenet/blob/01d65bb1409cb34a8bc1444b67dd1ab011d6d4fa/src/Lucene.Net/Support/WeakDictionary.cs]). An effort to port {{ConditionalWeakTable}} from .NET Standard 2.1 back to .NET Standard 2.0 (LUCENENET-636) is currently underway, but stalled on [this J2N branch|https://github.com/NightOwl888/J2N/tree/feature/conditional-weak-table/src/J2N/Runtime/CompilerServices]. Unfortunately, it depends on unmanaged resources that somehow need to be re-mapped or embedded in order to make it functional. Perhaps there is also a way to cut through at a lower level and make a {{ConditionalWeakIdentityTable}} that could be used as a direct replacement for {{WeakIdentityMap}} instead of using {{IdentityWeakReference}}.

If you could take a look at using {{ConditionalWeakTable}} to solve this issue, it would be much appreciated. Since there appears to be one place where the enumerator is required [here|https://github.com/apache/lucenenet/blob/01d65bb1409cb34a8bc1444b67dd1ab011d6d4fa/src/Lucene.Net/Store/ByteBufferIndexInput.cs#L377], the best approach would be to first check for compatibility on .NET Standard 2.1 and if that works, help us to complete the port of {{ConditionalWeakTable}} for .NET Framework 4.5 and .NET Standard 2.0 by submitting a PR to [the J2N project|https://github.com/NightOwl888/J2N/tree/feature/conditional-weak-table] so the same fix can also be applied to those platforms.



> Sequential IndexWriter performance in concurrent environments.
> --------------------------------------------------------------
>
>                 Key: LUCENENET-640
>                 URL: https://issues.apache.org/jira/browse/LUCENENET-640
>             Project: Lucene.Net
>          Issue Type: Bug
>          Components: Lucene.Net Core
>    Affects Versions: Lucene.Net 4.8.0
>            Reporter: Mathias Henriksen
>            Priority: Major
>              Labels: performance
>             Fix For: Lucene.Net 4.8.0
>
>         Attachments: AssertFinalBug.jpg, IdentityWeakReferenceBug.jpg, Program.cs, overviewBug.jpg
>
>          Time Spent: 20m
>  Remaining Estimate: 0h
>
> When creating Lucene.Net indices in parallel, sequential-like performance is experienced. Profiling 8 concurrent IndexWriter instances writing in parallel shows that WeakIdentityMap::IdentityWeakReference::Equals spends most time garbage collecting (94.91%) and TokenStream::AssertFinal (87.09% garbage collecting) in my preliminary tests (see screenshots).
> The [WeakIdentityMap|https://github.com/apache/lucenenet/blob/master/src/Lucene.Net/Util/WeakIdentityMap.cs] implementation uses an IdentityWeakReference as key, which is implemented as a class. By inspection of this class, it is merely a System.Runtime.InteropServices.GCHandle wrapper as can be seen in the mono project, manually wrapping of this struct in a struct rather than a class - will eliminate some of the immense amounts of garbage collection.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)