[GitHub] [lucenenet] Shazwazza commented on issue #308: Investigate slow test: Lucene.Net.Tests.Index.TestAddIndexes::TestAddIndexesWithCloseNoWait()

[GitBox <gi...@apache.org>]

Shazwazza commented on issue #308:
URL: https://github.com/apache/lucenenet/issues/308#issuecomment-666122407


   Just adding some notes here for now, with the change here https://github.com/apache/lucenenet/pull/312 this fixes a deadlocking scenario which i was encountering when trying to debug this. Now that there is no deadlock the tests will work as expected however using a custom nunit test runner attribute (FindFirstFailingSeed) I still see that the test runs slow. While debugging it and stepping through the code we no longer see deadlocks so I can actively step through the code but when using the Parallel Stacks view in VS or just the Threads window we can see there's always many threads waiting on a single lock. 
   
   Some main things to note:
   
   * Quite a lot of threads call into IndexWriter.CopySegmentAsIs and then wait on
     * `Debug.Assert(!SlowFileExists(directory, newFileName), "file \"" + newFileName + "\" already exists; siFiles=" + string.Format(J2N.Text.StringFormatter.InvariantCulture, "{0}", siFiles));`  ... Since this only runs in Debug builds, i wonder if the unit tests on the build server are running in debug mode? This assertion actually comes with a lot of overhead!! It also takes a lock on the same lock that we are waiting on within `MockDirectoryWrapper.locker`. 
     * And then later on `info.Info.Dir.Copy(directory, file, newFileName, context);` ... This also takes a lock on the same lock we are waiting within `MockDirectoryWrapper.locker` because it calls into `MockDirectoryWrapper.Copy` which takes a lock 
       * ... Actually, it turns out that `MockDirectoryWrapper.Copy` will force a recursive lock on `MockDirectoryWrapper.locker` even with our changes/fixes to recursive locking. This is because: `MockDirectoryWrapper.Copy` (Lock!) -> `Directory.Copy` -> `IOUtils.DisposeWhileHandlingException(priorException, os, @is)` (where os == MockIndexOutputWrapper) -> `MockIndexOutputWrapper.Dispose` -> `MockDirectoryWrapper.RemoveIndexOutput` (Recursive lock!)
   
   I think the above few things probably plays a role in the performance of this test so need to investigate this a little more. Since there is a recursive lock via indirect references that general means there can be deadlocks again but seeing as though there are so many locks trying to be taken anyways this will slow things down quite a lot.
       
   
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org