[jira] [Commented] (DRILL-5070) Code gen: create methods in fixed order to allow test verification

["ASF GitHub Bot (JIRA)" <ji...@apache.org>]

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

ASF GitHub Bot commented on DRILL-5070:
---------------------------------------

GitHub user paul-rogers opened a pull request:

    https://github.com/apache/drill/pull/684

    DRILL-5070: Code gen: create methods in fixed order to allow test verification	

    A handy technique in testing is to compare generated code against a "golden" copy that defines the expected results. However, at present, Drill generates code using the method order returned by Class.getDeclaredMethods, but this method makes no guarantee about the order of the methods. The order varies from one run to the next. There is some evidence this link that order can vary even within a single run, though a quick test was unable to reproduce this case.
    
    The fix is simple, in the SignatureHolder constructor, sort methods by name after retrieving them from the class. The sort ensures that method order is deterministic. Fortunately, the number of methods is small, so the sort step adds little cost.

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/paul-rogers/drill DRILL-5070

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/drill/pull/684.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #684
    
----
commit 63d9b5aaffb6982355c785efe26dfede81c0a66a
Author: Paul Rogers <pr...@maprtech.com>
Date:   2016-11-28T05:51:02Z

    DRILL-5070: Actual fix
    
    The fix: order methods by name in the “signature” so that generated
    methods occur in the same order every time.

commit b25bbf5c25c23c196710af0e72b79cc97acff0e9
Author: Paul Rogers <pr...@maprtech.com>
Date:   2016-11-28T05:52:41Z

    DRILL-5070: Test cases
    
    Modifies the existing ExpressionTest to capture generated code and
    compare it to a “golden” copy. Provides the mechanism to do the
    comparison.
    
    Before the fix, comparisons randomly fail due to random method order.
    After the fix, the comparisons consistently work, showing that code is
    generated in the same order each time.

----


> Code gen: create methods in fixed order to allow test verification
> ------------------------------------------------------------------
>
>                 Key: DRILL-5070
>                 URL: https://issues.apache.org/jira/browse/DRILL-5070
>             Project: Apache Drill
>          Issue Type: Bug
>    Affects Versions: 1.8.0
>            Reporter: Paul Rogers
>            Priority: Minor
>
> A handy technique in testing is to compare generated code against a "golden" copy that defines the expected results. However, at present, Drill generates code using the method order returned by {{Class.getDeclaredMethods}}, but this method makes no guarantee about the order of the methods. The order varies from one run to the next. There is some evidence [this link|http://stackoverflow.com/questions/28585843/java-reflection-getdeclaredmethods-in-declared-order-strange-behaviour] that order can vary even within a single run, though a quick test was unable to reproduce this case.
> If method order does indeed vary within a single run, then the order can impact the Drill code cache since it compares the sources from two different generation events to detect duplicate code.
> This issue appeared when attempting to modify tests to capture generated code for comparison to future results. Even a simple generated case from {{ExpressionTest.testBasicExpression()}} that generates {{if(true) then 1 else 0 end}} (all constants) produced methods in different orders on each test run.
> The fix is simple, in the {{SignatureHolder}} constructor, sort methods by name after retrieving them from the class. The sort ensures that method order is deterministic. Fortunately, the number of methods is small, so the sort step adds little cost.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)