drill-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "ASF GitHub Bot (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (DRILL-5116) Enable generated code debugging in each Drill operator
Date Fri, 06 Jan 2017 00:13:58 GMT

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

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

Github user paul-rogers commented on a diff in the pull request:

    https://github.com/apache/drill/pull/716#discussion_r94879149
  
    --- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/expr/ClassGenerator.java
---
    @@ -374,6 +388,124 @@ public HoldingContainer declare(MajorType t, boolean includeNewInstance)
{
         return this.workspaceVectors;
       }
     
    +  /**
    +   * Prepare the generated class for use as a plain-old Java class
    +   * (to be compiled by a compiler and directly loaded without a
    +   * byte-code merge. Three additions are necessary:
    +   * <ul>
    +   * <li>The class must extend its template as we won't merge byte
    +   * codes.</li>
    +   * <li>A constructor is required to call the <tt>__DRILL_INIT__</tt>
    +   * method. If this is a nested class, then the constructor must
    +   * include parameters defined by the base class.</li>
    +   * <li>For each nested class, create a method that creates an
    +   * instance of that nested class using a well-defined name. This
    +   * method overrides the base class method defined for this purpose.</li>
    +   */
    +
    +  public void preparePlainJava() {
    +
    +    // If this generated class uses the "straight Java" technique
    +    // (no byte code manipulation), then the class must extend the
    +    // template so it plays by normal Java rules for finding the
    +    // template methods via inheritance rather than via code injection.
    +
    +    Class<?> baseClass = sig.getSignatureClass();
    +    clazz._extends(baseClass);
    +
    +    // Create a constuctor for the class: either a default one,
    +    // or (for nested classes) one that passes along arguments to
    +    // the super class constructor.
    +
    +    Constructor<?>[] ctors = baseClass.getConstructors();
    +    for (Constructor<?> ctor : ctors) {
    +      addCtor(ctor.getParameterTypes());
    +    }
    +
    +    // Some classes have no declared constructor, but we need to generate one
    +    // anyway.
    +
    +    if ( ctors.length == 0 ) {
    +      addCtor( new Class<?>[] {} );
    +    }
    +
    +    // Repeat for inner classes.
    +
    +    for(ClassGenerator<T> child : innerClasses.values()) {
    +      child.preparePlainJava();
    +
    +      // If there are inner classes, then we need to generate a "shim" method
    +      // to instantiate that class.
    +      //
    +      // protected TemplateClass.TemplateInnerClass newTemplateInnerClass( args... )
{
    +      //    return new GeneratedClass.GeneratedInnerClass( args... );
    +      // }
    +      //
    +      // The name is special, it is "new" + inner class name. The template must
    +      // provide a method of this name that creates the inner class instance.
    +
    +      String innerClassName = child.clazz.name();
    +      JMethod shim = clazz.method(JMod.PROTECTED, child.sig.getSignatureClass(), "new"
+ innerClassName);
    +      JInvocation childNew = JExpr._new(child.clazz);
    +      Constructor<?>[] childCtors = child.sig.getSignatureClass().getConstructors();
    +      Class<?>[] params;
    +      if (childCtors.length==0) {
    +        params = new Class<?>[0];
    +      } else {
    +        params = childCtors[0].getParameterTypes();
    +      }
    +      for (int i = 1; i < params.length; i++) {
    +        Class<?> p = params[i];
    +        childNew.arg(shim.param(model._ref(p), "arg" + i));
    +      }
    +      shim.body()._return(childNew);
    +    }
    +  }
    +
    +  /**
    +   * The code generator creates a method called __DRILL_INIT__ which takes the
    +   * place of the constructor when the code goes though the byte code merge.
    +   * For Plain-old Java, we call the method from a constructor created for
    +   * that purpose. (Generated code, fortunately, never includes a constructor,
    +   * so we can create one.) Since the init block throws an exception (which
    +   * should never occur), the generated constructor converts the checked
    +   * exception into an unchecked one so as to not require changes to the
    +   * various places that create instances of the generated classes.
    +   *
    +   * Example:<code><pre>
    +   * public StreamingAggregatorGen1() {
    +   *       try {
    +   *         __DRILL_INIT__();
    +   *     } catch (SchemaChangeException e) {
    +   *         throw new UnsupportedOperationException(e);
    +   *     }
    +   * }</pre></code>
    +   *
    +   * Note: in Java 8 we'd use the <tt>Parameter</tt> class defined in Java's
    +   * introspection package. But, Drill prefers Java 7 which only provides
    +   * parameter types.
    +   */
    +
    +  private void addCtor(Class<?>[] parameters) {
    +    JMethod ctor = clazz.constructor(JMod.PUBLIC);
    +    JInvocation superCall = null;
    --- End diff --
    
    Fixed.


> Enable generated code debugging in each Drill operator
> ------------------------------------------------------
>
>                 Key: DRILL-5116
>                 URL: https://issues.apache.org/jira/browse/DRILL-5116
>             Project: Apache Drill
>          Issue Type: Improvement
>    Affects Versions: 1.9.0
>            Reporter: Paul Rogers
>            Assignee: Paul Rogers
>            Priority: Minor
>
> DRILL-5052 adds the ability to debug generated code. Some of the code generated by Drill's
operators has minor problems when compiled directly using the new technique. These issues
are ignore by the byte-code-merge technique uses in production. This ticket asks to try the
DRILL-5052 feature in each operator, clean up any minor problems, and ensure each operator
generates code suitable for debugging. Use the new {{CodeGenerator.plainOldJavaCapable()}}
method to mark each generated class as ready for "plain-old Java" code gen.
> The advantages of this feature are two:
> 1. Ability to step through the generated code to increase understanding of existing operators
and to ease development of improvements to existing operators and of any new operators we
choose to create.
> 2. Open the door to experimenting with how to improve performance of the generated code.



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

Mime
View raw message