[jira] [Closed] (LUCENE-5075) Wasted work in FailTwiceDuringMerge.eval()

["Adrien Grand (JIRA)" <ji...@apache.org>]

     [ https://issues.apache.org/jira/browse/LUCENE-5075?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Adrien Grand closed LUCENE-5075.
--------------------------------

    Resolution: Won't Fix

Hi Adrian. Although your patches actually prevent wasted work from being performed, I'm not going to merge them:
 - the first one makes the code harder to read although performance doesn't matter much (this is testing code),
 - the 2nd patch changes the behavior of the eval method given that {{doFail}} can be modified by concurrent threads.
                
> Wasted work in FailTwiceDuringMerge.eval()
> ------------------------------------------
>
>                 Key: LUCENE-5075
>                 URL: https://issues.apache.org/jira/browse/LUCENE-5075
>             Project: Lucene - Core
>          Issue Type: Bug
>    Affects Versions: 4.3.1
>         Environment: any
>            Reporter: Adrian Nistor
>              Labels: patch, performance
>         Attachments: patch2.diff, patch.diff
>
>
> The problem appears in version 4.3.1 and in revision 1495833.  I
> attached a one-line patch (patch.diff) that fixes it.  This problem
> and the attached patch are similar to some of the problems reported in
> LUCENE-5044 and their patch (patchAll.diff in LUCENE-5044).
> In method "FailTwiceDuringMerge.eval", the loop over "trace" should
> only be executed when at least one of "!didFail1" or "!didFail2" are
> "true".  The loop has no side effects when both "!didFail1" and
> "!didFail2" are "false" due to these two "if" statements:
> {code:java|borderStyle=solid}
> if (SegmentMerger.class.getName().equals(trace[i].getClassName()) && "mergeTerms".equals(trace[i].getMethodName()) && !didFail1) {
> {code}
> and
> {code:java|borderStyle=solid}
> if (LiveDocsFormat.class.getName().equals(trace[i].getClassName()) && "writeLiveDocs".equals(trace[i].getMethodName()) && !didFail2) {
> {code}
> A similar problem exists in "FailOnlyInSync.eval", where the loop over
> "trace" should only be executed when "doFail" is "true", because the
> loop has no side effect when "doFail" is "false" due to this "if"
> statement:
> {code:java|borderStyle=solid}
> if (doFail && MockDirectoryWrapper.class.getName().equals(trace[i].getClassName()) && "sync".equals(trace[i].getMethodName())) {
> {code}
> I attached a second patch (patch2.diff) for this second problem.  Note
> that the code already checks outside the loop if "doFail" is "true":
> {code:java|borderStyle=solid}
> if (doFail) {
> {code}
> and thus the patch does not need to check again (the patch only needs
> to delete the check inside the loop).

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org