lucenenet-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From GitBox <>
Subject [GitHub] [lucenenet] NightOwl888 commented on issue #346: Remove Debugging.AssertsEnabled
Date Sat, 17 Oct 2020 17:52:00 GMT

NightOwl888 commented on issue #346:

   > I think it is only truly save the allocation of a string when running with Debugging.Assert
= true.
   Correct. But if the application is running correctly, building the string will never occur.
The string should only be built when there is an error. Building the string on every call
and then discarding it slows down the tests.
   > Would it be fine to just remove this overload of Debugging.Assert completely?
   Well, no because there are 2-3 cases where exceptions will be thrown when trying to build
the string in the `true` case. I am not sure if the comments that indicate which calls to
`Debugging.Assert` fail randomly still exist, so you might have to go back before #326 in
the commit history to find them. However, I don't object to changing the ones that don't throw
exceptions when building the string to doing it inline. 
   I would prefer a better solution if there is one - in Java, the string is only built in
the case there is a failure, and it would be best to duplicate that so we don't have to deal
with these failure cases. However, in cases where performance is being significantly affected
in production, we can call the overload without the lambda.
   Frankly, I think the most performant solution would be just to eliminate the call to `Debugging.Assert`
and put the code inline.
   // Before
   if (Debugging.AssertsEnabled) Debugging.Assert(outputLen > 0, () => "output contains
empty string: " + scratchChars);
   // After
   if (Debugging.AssertsEnabled && !(outputLen > 0)) throw new AssertionException($"output
contains empty string: {scratchChars}");

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:

View raw message