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

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_r95468443
  
    --- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/compile/ClassBuilder.java ---
    @@ -64,36 +67,33 @@
      * local variables. Have fun!</li>
      * </ul>
      * <p>
    - * Note: not all generated code is ready to be compiled as plain-old
    - * Java. Some classes omit from the template the proper <code>throws</code>
    - * declarations. Other minor problems may also crop up. All are easy
    - * to fix. Once you've done so, add the following to mark that you've
    - * done the clean-up:<pre>
    - * cg.plainOldJavaCapable(true); // Class supports plain-old Java</pre>
    + * Most generated classes have been upgraded to support Plain-old Java
    + * compilation. Once this work is complete, the calls to
    + * <tt>plainOldJavaCapable<tt> can be removed as all generated classes
    + * will be capable.
      * <p>
    - * The setting to prefer plain-old Java is ignored for generated
    + * The setting to prefer plain-old Java is ignored for any remaining 
generated
      * classes not marked as plain-old Java capable.
      */
     
     public class ClassBuilder {
     
    -  public static final String SAVE_CODE_OPTION = CodeCompiler.COMPILE_BASE 
+ ".save_source";
       public static final String CODE_DIR_OPTION = CodeCompiler.COMPILE_BASE + 
".code_dir";
     
       private final DrillConfig config;
       private final OptionManager options;
    -  private final boolean saveCode;
       private final File codeDir;
     
       public ClassBuilder(DrillConfig config, OptionManager optionManager) {
         this.config = config;
         options = optionManager;
     
    -    // The option to save code is a boot-time option because
    -    // it is used selectively during debugging, but can cause
    -    // excessive I/O in a running server if used to save all code.
    +    // Code can be saved per-class to enable debugging.
    +    // Just mark the code generator as to be persisted,
    +    // point your debugger to the directory set below, and you
    --- End diff --
    
    Thanks. The key thing to remember here is that the generated code does not 
exist until the code generator runs, so it is somewhat hard to set a breakpoint 
ahead of time given the current structure.
    
    You are right that we can *add* another feature that causes the code to 
reuse class names so that we can set breakpoints inside the generated code. I 
worked around this in my sort performance stuff by creating my own copy of the 
code which I then hand modified to try out various changes. That was a quick 
way to avoid the name issue and to try changes without having to muck with the 
code generator just to do a prototype.
    
    Do you feel we need to add the consistent-class-name feature in this PR, or 
can that be a refinement in a later PR?
    
    And, I agree we should document usage as a way to encourage folks to try it 
and help us figure out what else we can improve.


> 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
>              Labels: ready-to-commit
>
> 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)

Reply via email to