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:
[email protected]


Reply via email to