FWD » Core Development » Conversion Tools

Use anonymous methods rather than anonymous classes to implement the many features in the P2J runtime which require inversion of control. This should significantly reduce the amount of emitted code necessary to implement business logic callbacks.

Instructions to upgrade to OpenJDK 8 on Ubuntu 15.04¶

To install the JDK and J2SE source (for debugging/review):

sudo apt-get install openjdk-8-jdk
sudo apt-get install openjdk-8-source

sudo update-alternatives --config java
sudo update-alternatives --config javac

sudo update-alternatives --get-selections | grep java

Next, set up a default-java symbolic link in /usr/lib/jvm.

cd /usr/lib/jvm

sudo rm default-java

sudo ln -s java-8-openjdk-amd64 default-java

cd /usr/lib
sudo rm libjvm.so
sudo ln -s /usr/lib/jvm/default-java/jre/lib/amd64/server/libjvm.so libjvm.so

The (non-commented portion of the) environment file should read as follows:

JAVA_HOME='/usr/lib/jvm/default-java'
CLASSPATH=''
LD_LIBRARY_PATH='$JAVA_HOME/jre/lib/amd64:$JAVA_HOME/jre/lib/amd64/server'

ldconfig -p | grep libjvm.so

libjvm.so (libc6,x86-64) => /usr/lib/libjvm.so

libjvm.so (libc6,x86-64) => /usr/lib/jvm/java-7-openjdk-amd64/jre/lib/amd64/server/libjvm.so
libjvm.so (libc6,x86-64) => /usr/lib/libjvm.so

sudo ldconfig

    linux-vdso.so.1 =>  (0x00007ffc91f19000)
    libcrypt.so.1 => /lib/x86_64-linux-gnu/libcrypt.so.1 (0x00007fb8aa1f2000)
    libjvm.so => /usr/lib/libjvm.so (0x00007fb8a926a000)
    libc.so.6 => /lib/x86_64-linux-gnu/libc.so.6 (0x00007fb8a8ea0000)
    libstdc++.so.6 => /usr/lib/x86_64-linux-gnu/libstdc++.so.6 (0x00007fb8a8b91000)
    libm.so.6 => /lib/x86_64-linux-gnu/libm.so.6 (0x00007fb8a8889000)
    libdl.so.2 => /lib/x86_64-linux-gnu/libdl.so.2 (0x00007fb8a8685000)
    libpthread.so.0 => /lib/x86_64-linux-gnu/libpthread.so.0 (0x00007fb8a8467000)
    /lib64/ld-linux-x86-64.so.2 (0x00007fb8aa42a000)
    libgcc_s.so.1 => /lib/x86_64-linux-gnu/libgcc_s.so.1 (0x00007fb8a8251000)

A note regarding PL/Java¶

At this time, we have not yet rebuilt PL/Java with Java 8. The version built with Java 7 continues to run without a problem under Java 8, and a p2jpl.jar file built under Java 8 will install and run properly in a database using a Java 8 runtime environment. We will investigate rebuilding PL/Java for Java 8 separately. If you need to retain the capability to build PL/Java from source, DO NOT remove Java 7 from your system until we have determined that it can be built with Java 8. However, DO follow the above steps to ensure OpenJDK 8 is the default for your system.

For those of us on Ubuntu 14_04, the openjdk-8 packages don't exist (haven't been backported - seems unlikely that they will be).

• build OpenJDK from source
• move to Ubuntu 15_04 (although we'll be making another move to 16_04 in April)
• use the Oracle JDK
• use Azul Zulu (OpenJDK built and hosted by Azul - I'd serve it from our binary repo)

In the very short term (today / tomorrow) I prefer using the Oracle JDK. I imagine in the demo timeframe we might look at using Azul Zulu.

Paul Eames wrote:

In the very short term (today / tomorrow) I prefer using the Oracle JDK.

FYI, I haven't done any runtime testing with the Oracle JDK, but I did build P2J with it. The default JavaDoc doclet in use seems a lot stricter than OpenJDK's default.

Addition to the instructions: p2jspi.jar should be copied to the new jre/lib/ext/.

Does it matter that ldd spawn reports different dependencies for me than in your note 3?

My output:

ldd spawn 
    linux-vdso.so.1 =>  (0x00007fffb7dfe000)
    libcrypt.so.1 => /lib/x86_64-linux-gnu/libcrypt.so.1 (0x00007f38de329000)
    libjvm.so => /usr/lib/libjvm.so (0x00007f38dd34f000)
    libc.so.6 => /lib/x86_64-linux-gnu/libc.so.6 (0x00007f38dcf89000)
    libm.so.6 => /lib/x86_64-linux-gnu/libm.so.6 (0x00007f38dcc83000)
    libdl.so.2 => /lib/x86_64-linux-gnu/libdl.so.2 (0x00007f38dca7f000)
    libpthread.so.0 => /lib/x86_64-linux-gnu/libpthread.so.0 (0x00007f38dc860000)
    /lib64/ld-linux-x86-64.so.2 (0x00007f38de56b000)

Interesting.

Eric's output had libstdc++.so which is the standard C++ class library. In other words, the ISO C++ standards specify a certain set of classes (e.g. string or vector) that all compliant implementations must provide. These are implemented in libstdc++.so. We don't write any C++ code, so I don't know why this is linked in to Eric's version.

https://gcc.gnu.org/onlinedocs/libstdc++/faq.html

Eric's output had libgcc_s.so which can be emitted on a machine-specific basis depending on CPU architecture. For example, it can provide some low-level math helpers that on some CPUs can be implemented directly using simple inline instructions. If Eric's CPU could not handle those cases, then these helpers might be required and the compiler would then emit calls to them that would be linked in via this library. On the other hand, I doubt Eric's CPU is incapable of these math features, so the more likely case is for exception handling. With the C++ library being included, the likely case is that the C++ exception handling pulled libgcc_s.so into the dependencies.

https://gcc.gnu.org/onlinedocs/gccint/Libgcc.html

Eric and I are both on Ubuntu 15.04. My ldd output is the same as his. With that said, if the spawner is working for you, then I would not worry about the missing libraries.

A quick (and incomplete) list of the candidates for lambda conversion:

• LogicalExpression
• WhileClause
• put_field (PutExpr)/export_field (ExportExpr) when these are complex expressions
• BufferValidator
• Validatable/Validator
• complex form header expressions
• complex embedded assignment expressions
• complex browse column expressions
• Block
• UI Trigger
• Database Trigger
• ToClause
• runtime_frame_options
• runtime_browse_options
• HandleExpr, func_in_handle_ref annotation
• MESSAGE stmt parameter list
• NO-ERROR
• accumulate stmt
• variable_def_anon_expr template usage
• by clause with complex sort
• client-side where (WhereExpression)
• client where substitution
• query substitution
• RecordBuffer.start/endBatch()
• FieldReference

Branch 1735a was merged to trunk as revision 11035. 1735a was archived.

This revision changes two cases of LogicalExpression usage in converted code into lambdas (deferred evaluation for right operand of OR or AND operator; WHILE clause in DO/FOR/REPEAT blocks). This eliminates a significant amount of generated code, leading to a cleaner/more readable result. It also removes a substantial number of anonymous inner classes (between 15% and 25% of classes in large projects). This update also removes non-compact mode support in the TRPL rule-sets.

1735a passed MAJIC conversion and runtime regression testing (only tc_job_clock_002 has failed multiple times, but that is a fragile test). The customer's GUI code also is passing conversion testing.

You should expect a VERY LARGE number of generated code changes for all converted code. Two examples:

OR/AND operators are much cleaner now:

<                   if (_or(isGreaterThanOrEqual(sel.getEndDate(), aDate), new LogicalExpression()
<                   {
<                      public logical execute()
<                      {
<                         return isUnknown(sel.getEndDate());
<                      }
<                   }))
---
>                   if (_or(isGreaterThanOrEqual(sel.getEndDate(), aDate), () -> isUnknown(sel.getEndDate())))

When such operators are combined in a single expression:

<                            if (_or(or(isGreaterThan(abs(date.differenceNum(period, dr.getDat())), 1), new LogicalExpression()
<                            {
<                               public logical execute()
<                               {
<                                  return and(and(cdays, new LogicalExpression()
<                                  {
<                                     public logical execute()
<                                     {
<                                        return isEqual(abs(date.differenceNum(period, dr.getDat())), 1);
<                                     }
<                                  }), new LogicalExpression()
<                                  {
<                                     public logical execute()
<                                     {
<                                        return or(isNotEqual(dr.getNum(), nextNum), new LogicalExpression()
<                                        {
<                                           public logical execute()
<                                           {
<                                              return isNotEqual(dr.getNum(), 1);
<                                           }
<                                        });
<                                     }
<                                  });
<                               }
<                            }), new LogicalExpression()
<                            {
<                               public logical execute()
<                               {
<                                  return and(and(cdays, new LogicalExpression()
<                                  {
<                                     public logical execute()
<                                     {
<                                        return isEqual(abs(date.differenceNum(period, dr.getDat())), 0);
<                                     }
<                                  }), new LogicalExpression()
<                                  {
<                                     public logical execute()
<                                     {
<                                        return isNotEqual(dr.getNum(), nextNum);
<                                     }
<                                  });
<                               }
<                            }))
---
>                            if (_or(or(isGreaterThan(abs(date.differenceNum(period, dr.getDat())), 1), () -> and(and(cdays, () -> isEqual(abs(date.differenceNum(period, dr.getDat())), 1)), () -> or(isNotEqual(dr.getNum(), nextNum), () -> isNotEqual(dr.getNum(), 1)))), () -> and(and(cdays, () -> isEqual(abs(date.differenceNum(period, dr.getDat())), 0)), () -> isNotEqual(dr.getNum(), nextNum))))

DO/REPEAT/FOR WHILE clauses:

<                doWhile("loopLabel3", new LogicalExpression(isOk), new Block()
---
>                doWhile("loopLabel3", () -> isOk, new Block()

Or if it is a more complex expression:

<             LogicalExpression whileClause0 = new LogicalExpression()
<            {
<               public logical execute()
<              {
<                  return isLessThan(startNum, endNum);
<               }
<            };
---
>            LogicalOp whileClause0 = () -> isLessThan(startNum, endNum);

In real applications I have seen the total number of converted classes be reduced by 15% to 25%. Although the raw performance of the implementation is roughly equivalent, it is expected that there may be benefit to the greatly reduced class loading, memory usage (and associated overhead).

Task branch 1735b has changes that modify conversion to emit UI validation expressions as lambdas instead of an inner class of the block. The runtime code is modified to accept lambdas. Validatable and Validator classes are now removed.

Both conversion and runtime regression (only tc_job_002 failed) testing passed. The converted code has significant expected changes. The following is an example.

Old:

         public void body()
         {
            editRangeRangeParmsFrame.widgetStartNum().setValidatable(new Validation0());
            ...
         }

         class Validation0
         extends Validator
         {
            public logical validateExpression(BaseDataType _newValue_)
            {
               final integer _startNum = (integer) _newValue_;
               return isGreaterThanOrEqual(_startNum, 0);
            }

            public character validateMessage(BaseDataType _newValue_)
            {
               return new character("Start of range must be non-negative.");
            }
         }

New:

editRangeRangeParmsFrame.widgetStartNum().setValidation(((ValidationExpr<integer>) (integer _startNum) -> isGreaterThanOrEqual(_startNum, 0)), "Start of range must be non-negative.");

There are 2 forms of the new CommonWidget.setValidation() method:

public void setValidation(ValidationExpr valexp, String valmsg);
public void setValidation(ValidationExpr valexp, ValidationMsg valmsg);

The first is used when the valmsg is a string literal. The second is used for all other cases.

Some validation expressions only access constants and the passed in new value. Such code will convert to a "naturally" non-capturing lambda. Such constructs are significantly faster than the original inner class approach. However, it is not clear to me if such a relative improvement can really be detected at the application level.

I've merged 1735b to trunk as revision 11038. Task branch 1735b has been archived.

There are significant changes to the manual MAJIC code. This is all checked in to git staging. On devsrv01, you must pull down the those MAJIC changes (cd ~/testing/majic && git pull origin staging). You must reconvert any 4GL code (devsrv01 or elsewhere) that has VALIDATION expression usage.

Changed conversion to emit DELETE statement validation expressions as lambdas instead of an inner class of the block. Changed conversion to emit complex PUT or EXPORT expressions (not literals, not simple scalar var refs, not field references) as lambdas instead of an inner class of the block. In all 3 cases the runtime code is modified to accept and process lambdas. The BufferValidator class was removed. Refs #1735.

Both conversion and runtime regression (only a couple of common false negatives occurred) testing passed. The converted code has significant expected changes.

DELETE Validation Expressions

Old way:

         public void body()
         {
            RecordBuffer.openScope(book);
            book.create();
            book.deleteRecord(new Validation0());
         }

         class Validation0
         extends BufferValidator
         {
            public logical validateExpression()
            {
               return new logical(true);
            }

            public character validateMessage()
            {
               return new character("");
            }
         }

New way:

book.deleteRecord(() -> new logical(true), "");

You can see uast/delete_validation.p for a slightly more complicated version.

There are 2 forms of the new BufferImpl.deleteRecord() method:

public logical deleteRecord(Supplier<logical> valexp, String valmsg)
public logical deleteRecord(Supplier<logical> valexp, Supplier<character> valmsg)

The first is used when the valmsg is a string literal. The second is used for all other cases.

Complex PUT/EXPORT Expressions

Old:

               sStream.put(new FieldEntry[]
               {
                  new PutField(new PutExpr0(), FMT_STR_1),
                  ...
               });
            }

            ...
         }

         class PutExpr0
         extends GenericExpression
         {
            public BaseDataType resolve()
            {
               return subscript(cdat, i);
            }
         }

New:

               sStream.put(new FieldEntry[]
               {
                  new PutField(() -> subscript(cdat, i), FMT_STR_1),
                  ...
               });

PutField has been modified to add constructors (for each Resolvable constructor there is now also one that accepts a Supplier<BaseDataType>):

   public PutField(Supplier<BaseDataType> val)
   public PutField(Supplier<BaseDataType> val, String fmt)
   public PutField(Supplier<BaseDataType> val, boolean starts, int col)
   public PutField(Supplier<BaseDataType> val, boolean starts, NumberType col)
   public PutField(Supplier<BaseDataType> val, String fmt, boolean starts, NumberType col)
   public PutField(Supplier<BaseDataType> val, String fmt, boolean starts, int col)

This same approach is used for complex EXPORT expressions and the runtime changes are in ExportField. Any use of literals, simple scalar vars or field references are unchanged (which is most of the use cases). In MAJIC, this does drop about 2000 classes out of the application jar (roughly 7% of the business logic jar).

Some expressions only access constants and the passed in new value. Such code will convert to a "naturally" non-capturing lambda. Such constructs are significantly faster than the original inner class approach. However, it is not clear to me if such a relative improvement can really be detected at the application level.

I've merged 1735c to trunk as revision 11041. Task branch 1735c was archived.

There are significant changes to the manual MAJIC code. This is all checked in to git staging. On devsrv01, you must pull down the those MAJIC changes (cd ~/testing/majic && git pull origin staging). You must reconvert any 4GL code (devsrv01 or elsewhere) that has DELETE VALIDATION expression usage or which has complex PUT/EXPORT expressions.

Greg, I really like the look of this.

Thanks. Yes, it is already making a difference. There is much more improvement to come.

Task branch 1735d is currently based from trunk 11041 (it needs a rebase to trunk 11044). As of 1735d branch rev 11044, the following features have been implemented:

1. Simple field references as the rvalue in an embedded assignment (SET/UPDATE statement) have been changed to emit as a FieldReference. The previous approach neglected this leaving a non-compilable result (and one that would not have worked properly even if it compiled).

set ...
    i = tt.num
    ...
    with frame f.

This used to convert to:

            FrameElement[] elementList0 = new FrameElement[]
            {
               ...
               new EmbeddedAssignment(i, tt.getNum()),
               ...
            };

            fFrame.set(elementList0);

Now it converts to this:

            FrameElement[] elementList0 = new FrameElement[]
            {
               ...
               new EmbeddedAssignment(i, new FieldReference(tt, "num")),
               ...
            };

            fFrame.set(elementList0);

This was a bug in the existing project.

2. Literals and simple variables as rvalues in embedded assignments are emitted directly instead of using a Resolvable. This is a simple code cleanup that has nothing to do with lambdas.

set i
    i = 3
    d = d2.

This used to convert to:

            FrameElement[] elementList0 = new FrameElement[]
            {
               new Element(i, frame0.widgeti()),
               new EmbeddedAssignment(i, new EmbeddedAssignmentExpr0()),
               new EmbeddedAssignment(d, new EmbeddedAssignmentExpr1())
            };

            frame0.set(elementList0);

            ...
         }

         class EmbeddedAssignmentExpr0
         extends GenericExpression
         {
            public BaseDataType resolve()
            {
               return new integer(3);
            }
         }

         class EmbeddedAssignmentExpr1
         extends GenericExpression
         {
            public BaseDataType resolve()
            {
               return d2;
            }
         }

Now it converts to:

            FrameElement[] elementList0 = new FrameElement[]
            {
               new Element(i, frame0.widgeti()),
               new EmbeddedAssignment(i, 3),
               new EmbeddedAssignment(d, d2)
            };

            frame0.set(elementList0);

3. Convert complex embedded assignment expressions to lambdas instead of inner classes.

set ...
    d = d2 * 5
    with frame f.

Used to convert to:

            FrameElement[] elementList0 = new FrameElement[]
            {
               ...
               new EmbeddedAssignment(d, new EmbeddedAssignmentExpr3())
            };

            fFrame.set(elementList0);
            ...
         }

         ...

         class EmbeddedAssignmentExpr3
         extends GenericExpression
         {
            public BaseDataType resolve()
            {
               return multiply(d2, 5);
            }
         }

Now it converts to:

            FrameElement[] elementList0 = new FrameElement[]
            {
               ...
               new EmbeddedAssignment(d, () -> multiply(d2, 5))
            };

            fFrame.set(elementList0);

4. Convert complex browse column expressions to lambdas instead of inner classes.

DEFINE BROWSE b1 QUERY q1
DISPLAY
        tt.f1
        tt.f2
        i * 30  /* complex expression will emit for deferred evaluation */
        tt.f3
        WITH 10 DOWN single.

This used to convert as:

            FrameElement[] elementList0 = new FrameElement[]
            {
               new Element(new FieldReference(tt, "f1"), f1Frame.widgetB1F1Column()),
               new Element(new FieldReference(tt, "f2"), f1Frame.widgetB1F2Column()),
               new HeaderElement(new ColumnExpr0(), f1Frame.widgetB1Expr4Column()),
               new Element(new FieldReference(tt, "f3"), f1Frame.widgetB1F3Column())
            };
            f1Frame.widgetB1().registerQuery(query0, elementList0);

            ...
         }

         class ColumnExpr0
         extends GenericExpression
         {
            public BaseDataType resolve()
            {
               return multiply(i, 30);
            }
         }

Now it converts as:

            FrameElement[] elementList0 = new FrameElement[]
            {
               new Element(new FieldReference(tt, "f1"), f1Frame.widgetB1F1Column()),
               new Element(new FieldReference(tt, "f2"), f1Frame.widgetB1F2Column()),
               new HeaderElement(() -> multiply(i, 30), f1Frame.widgetB1Expr4Column()),
               new Element(new FieldReference(tt, "f3"), f1Frame.widgetB1F3Column())
            };
            f1Frame.widgetB1().registerQuery(query0, elementList0);

5. Convert complex form header to lambdas instead of inner classes.

form header ...
            "Another " + txt format "x(14)" 
     with ...
          frame fr.

This used to convert to:

            FrameElement[] elementList0 = new FrameElement[]
            {
               ...
               new HeaderElement(new HeaderExpr0(), frFrame.widgetExpr3())
            };
            frFrame.registerHeader(elementList0);

            ...
         }

         class HeaderExpr0
         extends GenericExpression
         {
            public BaseDataType resolve()
            {
               return concat("Another ", txt);
            }
         }

            FrameElement[] elementList0 = new FrameElement[]
            {
               ...
               new HeaderElement(() -> concat("Another ", txt), frFrame.widgetExpr3())
            };
            frFrame.registerHeader(elementList0);

The runtime support classes (EmbeddedAssignment and HeaderElement) were modified to accept lambdas and to use them internally. With this change the are no more use cases of named inner classes that exist within the anonymous Block inner class instances (inner classes are still heavily used throughout the project, but named inner class members of blocks are now gone).

The resulting conversion in MAJIC and the customer GUI app causes substantial change and the elimination of thousands of inner classes.

This has passed MAJIC conversion and runtime regression testing (it took 2 passes, but only tc_job_002 was failing between the runs). CTRL-C testing has also passed. However, the current version does seem to cause MAJIC to run slower (the main regression runs in 210 to 215 minutes now, but before it was running in 195 minutes.

ETF has been at least partially tested using this build. Functionally it did not cause issues and there was no identifiable performance degradation.

Considering the potential performance impact on MAJIC, I am not planning to check this in at this time. I will do some further development work to eliminate the issue.

Based on seeing the potential negative performance impact on MAJIC, I have done a less naive testcase to see if I can measure the performance problem using a converted 4GL testcase (instead of the naive hand-written testcase I've done in note 9.

def var txt as char init "NOT DISPLAYED".
def var i   as integer no-undo.
def var begin as int.
def var done  as int.

form header "A form header literal!" 
            space(30)
            "Another " + txt format "x(14)" 
     with title "Complex Header Expressions In a Loop" 
          no-labels
          down
          frame fr.
form i with frame fr.

def stream rpt.
output stream rpt to ./sample_form_header_report.txt.

txt = "Header".

begin = etime.

do i = 1 to 50000:
   display stream rpt i with frame fr.
   down stream rpt with frame fr.
end.

done = etime.

output stream rpt close.

message "Main loop executed in " + string(done - begin) + " milliseconds.".

I then ran this with 5 different conversion strategies.

1. Named Inner Classes of the Block (same as P2J Trunk)

            FrameElement[] elementList0 = new FrameElement[]
            {
               new HeaderElement("A form header literal!", frFrame.widgetExpr1()),
               new HeaderElement(new HeaderExpr0(), frFrame.widgetExpr3())
            };
            frFrame.registerHeader(elementList0);

            ...
         }

         class HeaderExpr0
         extends GenericExpression
         {
            public BaseDataType resolve()
            {
               return concat("Another ", txt);
            }
         }

2. Anonymous inner class

            FrameElement[] elementList0 = new FrameElement[]
            {
               new HeaderElement("A form header literal!", frFrame.widgetExpr1()),
               new HeaderElement(new GenericExpression()
               {
                  public BaseDataType resolve()
                  {
                     return concat("Another ", txt);
                  }
               }, frFrame.widgetExpr3())
            };
            frFrame.registerHeader(elementList0);

3. Capturing lambda

            FrameElement[] elementList0 = new FrameElement[]
            {
               new HeaderElement("A form header literal!", frFrame.widgetExpr1()),
               new HeaderElement(() -> concat("Another ", txt), frFrame.widgetExpr3())
               // new HeaderElement(() -> concat("Another ", "Header"), frFrame.widgetExpr3())
               new HeaderElement((Object[] args) -> concat("Another ", (character) args[0]), frFrame.widgetExpr3(), txt)
            };
            frFrame.registerHeader(elementList0);

4. Naturally non-capturing lambda (using literal text instead of a variable reference)

            FrameElement[] elementList0 = new FrameElement[]
            {
               new HeaderElement("A form header literal!", frFrame.widgetExpr1()),
               new HeaderElement(() -> concat("Another ", "Header"), frFrame.widgetExpr3())
            };
            frFrame.registerHeader(elementList0);

5. Non-capturing lambda (passing variable args to the HeaderElement which are then passed as lambda parameters to a refactored expression.

            FrameElement[] elementList0 = new FrameElement[]
            {
               new HeaderElement("A form header literal!", frFrame.widgetExpr1()),
               new HeaderElement((Object[] args) -> concat("Another ", (character) args[0]), frFrame.widgetExpr3(), txt)
            };
            frFrame.registerHeader(elementList0);

Results

Each set of numbers is (total elapsed time in ms for the test / total time spent executing the complex header expression in nanoseconds / average time spent executing the complex header expression in nanoseconds across 50000 iterations).

Findings

1. Although this test is less naive than the simple test in note 9, this latest attempt was NOT successful in duplicating any user-measurable change (in seconds of elapsed time) in the overall test execution based on the use of lambdas versus inner classes.
2. The variability of the overall test execution time ranged from 84.7 seconds to 96.1 seconds.
3. The fastest overall test execution time (84.7 seconds) occurred for the capturing lambdas (which is the slowest of the expression execution approaches).
4. The slowest overall test execution time (96.1 seconds) occurred for the naturally non-capturing lambdas (which is the fastest of the expression execution approaches).
5. The cumulative difference executing the expression over 50000 iterations was only 5.7 milliseconds on the Fresh Server Run 1 between the trunk approach of named inner classes (53920187 nanos or 53.9 ms) and the 1735d rev 11044 approach of capturing lambdas (59573074 nanos or 59.6 ms). In other words, when the overall variability of the testing may swing +/- 6 seconds, the difference in HeaderExpression implementation accounted for by capturing lambdas was not significant.
6. Lambdas do amortize very well. If you leave the server running and hit that same code path again, the cost of each call will decrease such that lambdas are less expensive than inner classes.
7. Non-capturing lambdas using args is pretty close (sometimes faster than) named inner classes.
8. Naturally non-capturing lambdas are very fast, and are much faster (by up to 30 milliseconds cumulative over 50000 iterations) than all other cases including inner classes. But this is still so relatively small that it essentially cannot be seen by a user (it is far smaller than the natural execution variability of the test itself).

With just these results in mind, so far, I cannot justify reworking the lambda comversion to use the non-capturing arguments approach. Next I will try a much more heavily nested set of captured references mixed with a more complicated lambda expression. If I can't show any difference there, then the previous potential performance issue may have been bogus or may be something unrelated to lambdas.

I have made an additional effort to create a testcase which could demonstrate some user-measurable performance degradation. After several rounds, I came up with this:

def var txt   as char init "NOT DISPLAYED".
def var i     as integer no-undo.
def var num   as integer no-undo. 
def var begin as int.
def var done  as int.

def stream rpt.
output stream rpt to ./sample_form_header_report.txt.

txt = "Header".

begin = etime.
repeat:
   do transaction:
      def var num2 as int init 100.
      repeat:
         do transaction:
            def var num3 as int init 200.
            repeat:
               def var num4 as int init 300.
               do transaction:
                  repeat:
                     num = 30.
                     repeat:
                        form header "A form header literal!" 
                                    space(30)
                                    "Another " + txt + string(num) + " " + string(num2) + " " + string(num3) + " " + string(num4)
                             with title "Complex Header Expressions In a Loop" 
                                  no-labels
                                  down
                                  frame fr.
                        form i with frame fr.

                        do transaction:
                           do i = 1 to 20000:
                              run loop-body.
                           end.
                        end.
                        leave.
                     end.
                     leave.
                  end.
               end.
               leave.
            end.
         end.
         leave.
      end.
   end.
   leave.
end.

done = etime.

output stream rpt close.

message "Main loop executed in " + string(done - begin) + " milliseconds.".

procedure loop-body:
   display stream rpt i with frame fr.
   down stream rpt with frame fr.
end.

Notice that this test has a very deeply nested set of anonymous Block instances and a complex expression that accesses multiple resources. Vars referenced in a form header are "promoted" to be members of the containing business logic class. But I manually edited the resulting code to move those vars into multiple different locations in the nested block hierarchy. In the result, I use System.nanoTime() to calculate the time spend executing lambdas. Although in my testcase, over 20000 iterations the capturing lambdas are slower than the original named inner class case, the difference is only measured in a cumulative tens of milliseconds. This cannot be seen at all in the overall elapsed time (the capturing lambda approach was "faster" than the named inner class approach, about half the time. The difference in overall timings is measured in hundreds of milliseconds to multiple seconds, depending on the run. The bottom line is that this result duplicates my findings from note 17, but it does so with a scenario that was meant to be as bad for the lambda case as is possible.

Based on this, I can't see any justification for using the non-capturing arguments approach at this time. I'm going to make some additional improvements to 1735d and then get it tested and merged to trunk.

With revision 11050 of 1735d, I've made changes to reduce the unwrapping needed for the entered and not entered 4GL "builtin functions".

The old way:

            if ((frame0Frame.widgetStr().isEntered()).booleanValue())
            {
               ...
            }
            if ((frame0Frame.widgetStr().isNotEntered()).booleanValue())
            {
               ...
            }

New way:

            if (frame0Frame.widgetStr()._isEntered())
            {
               ...
            }
            if (frame0Frame.widgetStr()._isNotEntered())
            {
               ...
            }

It is just a minor improvement that occurs for control flow use cases.

Branch 1735d was rebased from trunk 11051. The latest revision is now 11058 (it includes some revisions with changes for #3054).

I'm starting regression testing on this revision. Previous regression testing runs passed but the recent changes do require retesting.

Regression testing for 1735d revision 11058 has passed.

Branch 1735d has been rebased from trunk revision 11053. The latest revision is now 11060. Since there are no conflicts with the latest rebase, I am not going to re-run regression testing.

Branch 1735d has been merged to trunk as revision 11054. 1735d was archived.

Branch 1735e was created from P2J trunk revision 11071.

Ovidiu:

In 1735e revision 11076, I am currently reworking all use of WhereExpression to replace it with a lambda (in this case, a Supplier<logical>).

This means that code that used to look like this (this is from testcases/uast/client-where-user-defined-function.p):

      externalProcedure(new Block()
      {
         WhereExpression whereExpr0 = new WhereExpression()
         {
            public logical evaluate(final BaseDataType[] args)
            {
               return isEqual(hotMess(somethinElse.getSomeTxt()), whateva.getNum());
            }
         };

         WhereExpression whereExpr1 = new WhereExpression()
         {
            public logical evaluate(final BaseDataType[] args)
            {
               return isEqual(hotMess(new character(valueOf(length(somethinElse.getSomeTxt())))), whateva.getNum());
            }
         };

         public void body()
         {
...

            forEach("loopLabel0", new Block()
            {
               CompoundQuery query0 = null;

               public void init()
               {
                  frame0.openScope();
                  query0 = new CompoundQuery(false, false);
                  query0.addComponent(new AdaptiveQuery(somethinElse, (String) null, null, "somethinElse.id asc"));
                  query0.addComponent(new AdaptiveQuery(whateva, (String) null, whereExpr0, "whateva.id asc"));
               }

               public void body()
               {
                  query0.iterate();

Now will look like this:

      externalProcedure(new Block()
      {
         public void body()
         {
...

            forEach("loopLabel0", new Block()
            {
               CompoundQuery query0 = null;

               public void init()
               {
                  frame0.openScope();
                  query0 = new CompoundQuery(false, false);
                  query0.addComponent(new AdaptiveQuery(somethinElse, (String) null, null, "somethinElse.id asc"));
                  query0.addComponent(new AdaptiveQuery(whateva, (String) null, () -> isEqual(hotMess(somethinElse.getSomeTxt()), whateva.getNum()), "whateva.id asc"));
               }

               public void body()
               {
                  query0.iterate();

We are no longer emitting the WhereExpression inner class and instead of using a reference like whereExpr0 we emit a lambda directly.

I need to update RuntimeJastInterpreter, to replace its usage of WhereExpression with the new lambda approach. My planned approach is to add a SupplierLogicalLambd@a subclass similar to @LogicalLambda and to use an annotation to detect the difference between LogicalOp and Supplier<logical> in the evalExpression() code that processes JavaTokenTypes.LAMBDA. Of course, I'll also remove references to WhereExpression and code for WhereExpressionAdapter. Any thoughts on my approach are welcome.

I need a testcase or testcases that would have previously generated the use of WhereExpression in dynamic queries. Can you point me to such a testcase? Without it I can't test my changes.

Please update the testcases repo. I just added uast/dynamic-queries/p2488-a.p. It contains 3 cases of queries that require at this moment WhereExpression internal classes. I usually put both static and dynamic form in order to know what to expect and compare codes at runtime.

The greatest difficulty will be to determine the correct interface for the interpreter. At this moment only two cases are handled: P2JQuery.Parameter if the cast is used and LogicalOp otherwise. Normally the parent signature should be checked and the interface obtained dynamically, but I estimate that this is too slow. If you can 'leave' some annotation with desired functional interface, that would be nice.

Task branch 1735e was rebased from trunk rev 11082. The latest revision is now 11089.

I have updated uast/dynamic-queries/p2488-a.p. The code did not run in the 4GL (there were both compile problems and runtime problems).

I have checked in the final updates to finish client-where clause support. The dynamic query processing runtime conversion now supports the new lambda approach.

Task branch 1735e was rebased from trunk rev 11083. The latest revision is now 11091.

Task branch 1735e was rebased from trunk rev 11085. The latest revision is now 11093.

With revision 11095, branch 1735e now converts the "emitdef" query substitution parameters into lambda expressions. The use case is for multi-table non-preselect queries, on subsequent components (not the first component) where there are complex expressions or case-insensitive character vars that must be resolved more than once. The first instance of such sub-expressions of a where clause were previously marked with an "emitdef" annotation and this caused them to emit as an inner class as part of the current top-level block's instance members (function, internal proc, trigger or external proc). Within that same top-level block, any duplicate use of that same sub-expression would reuse the same instance of the anonymous inner class rather than emitting again.

Although this is not hard to write a testcase that causes this behavior, in actual usage, this is not very common. A good example looks something like this (see testcases/uast/delegated_evaluation_of_query_terms.p):

def var num as int init 30 format "999".
def var txt as char init "good".
def var dat as date init today.

def temp-table tt-first
   field num1 as int format "999" 
   field txt1 as char
   field dat1 as date.

def temp-table tt-second
   field num2 as int format "999" 
   field txt2 as char    
   field dat2 as date.

create tt-first.
num1 = 30.
txt1 = "bad".
dat1 = today.
create tt-first.
num1 = 40.
txt1 = "bad".
dat1 = today.
create tt-first.
num1 = 50.
txt1 = "bad".
dat1 = today.
create tt-first.
num1 = 30.
txt1 = "good".
dat1 = today + 1.
create tt-first.
num1 = 40.
txt1 = "good".
dat1 = today  + 1.
create tt-first.
num1 = 50.
txt1 = "good".
dat1 = today + 1.
create tt-first.
num1 = 30.
txt1 = "good".
dat1 = today.
create tt-first.
num1 = 40.
txt1 = "good".
dat1 = today.
create tt-first.
num1 = 50.
txt1 = "good".
dat1 = today.

create tt-second.
num2 = 30.
txt2 = "bad".
dat2 = today.
create tt-second.
num2 = 40.
txt2 = "bad".
dat2 = today.
create tt-second.
num2 = 50.
txt2 = "bad".
dat2 = today.
create tt-second.
num2 = 30.
txt2 = "good".
dat2 = today + 1.
create tt-second.
num2 = 40.
txt2 = "good".
dat2 = today  + 1.
create tt-second.
num2 = 50.
txt2 = "good".
dat2 = today + 1.
create tt-second.
num2 = 30.
txt2 = "good".
dat2 = today.
create tt-second.
num2 = 40.
txt2 = "good".
dat2 = today.
create tt-second.
num2 = 50.
txt2 = "good".
dat2 = today.

for each tt-first where dat1 eq today and num1 > (num - 1) and txt1 eq txt and txt ne "",
    each tt-second where dat2 eq today and num2 > (num + 1) and txt2 eq txt and txt ne "":

   if num2 = 50 and dat2 = today then txt = if txt eq "good" then "bad" else "good".

   display num txt dat
           tt-first.num1  tt-first.txt1 tt-first.dat1
           tt-second.num2 tt-second.txt2 tt-second.dat2
           with frame f-bogus down.
   down with frame f-bogus.

end.

/* results */

/*
┌───────────────────────────────────────────────────────────────────┐
│num txt      dat      num1 txt1     dat1     num2 txt2     dat2    │
│─── ──────── ──────── ──── ──────── ──────── ──── ──────── ────────│
│030 good     08/23/16  030 good     08/23/16  040 good     08/23/16│
│030 bad      08/23/16  030 good     08/23/16  050 good     08/23/16│
│030 bad      08/23/16  040 good     08/23/16  040 bad      08/23/16│
│030 good     08/23/16  040 good     08/23/16  050 bad      08/23/16│
│030 good     08/23/16  050 good     08/23/16  040 good     08/23/16│
│030 bad      08/23/16  050 good     08/23/16  050 good     08/23/16│
│                                                                   │
│                                                                   │
│                                                                   │
│                                                                   │
│                                                                   │
│                                                                   │
│                                                                   │
│                                                                   │
│                                                                   │
│                                                                   │
│                                                                   │
└───────────────────────────────────────────────────────────────────┘

Procedure complete. Press space bar to continue.
*/

The previous conversion looked like this (look at the qSubExprXX members and how they are being used):

      externalProcedure(new Block()
      {
         ...

         DateExpression qSubExpr0 = new DateExpression()
         {
            public date execute()
            {
               return date.today();
            }
         };

         Int64Expression qSubExpr1 = new Int64Expression()
         {
            public int64 execute()
            {
               return plus(num, 1);
            }
         };

         CharacterExpression qSubExpr2 = new CharacterExpression()
         {
            public character execute()
            {
               return toUpperCase(txt);
            }
         };

         LogicalExpression qSubExpr3 = new LogicalExpression()
         {
            public logical execute()
            {
               return isNotEqual(txt, "");
            }
         };

         public void body()
         {
            ...

            forEach("loopLabel0", new Block()
            {
               CompoundQuery query0 = null;

               public void init()
               {
                  fBogusFrame.openScope();
                  query0 = new CompoundQuery(false, false);
                  query0.addComponent(new AdaptiveQuery(ttFirst, "ttFirst.dat1 = ? and ttFirst.num1 > ? and upper(ttFirst.txt1) = ? and ?", null, "ttFirst.id asc", new Object[]
                  {
                     (P2JQuery.Parameter) () -> date.today(),
                     minus(num, 1),
                     toUpperCase(txt),
                     isNotEqual(txt, "")
                  }));
                  query0.addComponent(new AdaptiveQuery(ttSecond, "ttSecond.dat2 = ? and ttSecond.num2 > ? and upper(ttSecond.txt2) = ? and ?", null, "ttSecond.id asc", new Object[]
                  {
                     qSubExpr0,
                     qSubExpr1,
                     qSubExpr2,
                     qSubExpr3
                  }));
               }

Here is the lambda version using rev 11095 of 1735e:

            forEach("loopLabel0", new Block()
            {
               CompoundQuery query0 = null;

               public void init()
               {
                  fBogusFrame.openScope();
                  query0 = new CompoundQuery(false, false);
                  query0.addComponent(new AdaptiveQuery(ttFirst, "ttFirst.dat1 = ? and ttFirst.num1 > ? and upper(ttFirst.txt1) = ? and ?", null, "ttFirst.id asc", new Object[]
                  {
                     (P2JQuery.Parameter) () -> date.today(),
                     minus(num, 1),
                     toUpperCase(txt),
                     isNotEqual(txt, "")
                  }));
                  query0.addComponent(new AdaptiveQuery(ttSecond, "ttSecond.dat2 = ? and ttSecond.num2 > ? and upper(ttSecond.txt2) = ? and ?", null, "ttSecond.id asc", new Object[]
                  {
                     (DateExpr) () -> date.today(),
                     (Int64Expr) () -> plus(num, 1),
                     (CharacterExpr) () -> toUpperCase(txt),
                     (LogicalExpr) () -> isNotEqual(txt, "")
                  }));
               }

There is a new set of Resolvable sub-interfaces that I have added to the persist package. These all omit the resolve() method but they each (CharacterExpr, DateExpr...) have a public default Class getType() method that makes each one the equivalent of a functional interface. This allows us to emit a well-typed lambda that properly implements Resolvable. The current implementations of CharacterExpression, DateExpression and so forth are adapter classes and thus they cannot be used as a functional interface.

By keeping to the Resolvable interface, the runtime did not need much change to match this new implementation. The only exception is that the dynamic query interpreter (RuntimeJastInterpreter) did require rework to handle these new lambdas. I also fixed some missing support for the ANON_CTOR cases but it is not actually clear if that code is even needed anymore. If it is really not needed, then we should delete it. The only thing I can think of that might still use it is the support for complex BY clauses, but I simply don't know if that code can be hit by the interpreter. I chose to be safe rather than sorry.

Eric/Ovidiu: I would appreciate if you would each do a code review of this branch. There is quite a bit of change in here, even if they don't actually get used in that many places in the real customer converted code.

By the way, I have tested the new support using standalone testcases and I cannot find any problems. I think that the result is functionally equivalent to the prior support.

Eric/Ovidiu: I would appreciate if you would each do a code review of this branch. There is quite a bit of change in here, even if they don't actually get used in that many places in the real customer converted code.

Greg, I reviewed the 1735e/11095. The changes related to dynamic queries are sound. There is a strange duplication of ___Expr / ___Expression classes. As you documented, the latter ones probably can be dropped as the old-fashioned WhereExpression are already dropped and at this moment, RuntimeJastInterpreter is only used for dynamic queries. I initially implemented the RuntimeJastInterpreter thinking to process all kind of JAST trees. In this case the anonymous ___Expression constructors would have the chance to be used. At this moment I don't think P2J will need such dynamic processor unless some strange feature is introduced in future versions of P4GL.
I understand that the new __Expr functional interfaces are to be used with lambdas of their respective types. BTW they all have a small typo in class javadoc (<<p>).
I also passed through all other changes. I think they are fine, in fact the TRPL code is simplified as the resulted generated one. I leave Eric have the last word here. I hardly wait the new code pushed to P2J trunk.

BTW they all have a small typo in class javadoc (<<p>).

Thanks! It is now fixed.

Code review 1735e/11096:

This is an exciting improvement, even though the client-side where clause functionality (thankfully) isn't too common. The query substitution expression inner class stuff is more prevalent, and it will be good to have a tighter version of that in the converted code. That was some pretty impenetrable code you navigated. Nice work!

I didn't see anything concerning. I was a bit surprised how much complexity you found to remove in the where expression rules, though I guess that goes back to our discussions about what was really necessary to support in the current version of this feature set.

I didn't understand the TODO in control_flow.rules. I fixed a minor typo in database_access.rules, nothing functional, and committed it to rev 11097.

What still uses the WhereExpression class after these changes? The comment in the file header suggests there is still a use case.

I didn't understand the TODO in control_flow.rules.

That is OK. :)

What still uses the WhereExpression class after these changes? The comment in the file header suggests there is still a use case.

It isn't needed. I have removed it in revision 11098.

Rebased from trunk revision 11086. The latest revision is now 11099.

Constantin:

I am trying to eliminate all usage of the defctorid (the annotation which allows us to target the anonymous Block instance's default inititializer. This is part of the steps I am taking to move our code blocks to a lambda model (#1735).

The frame_construction.rules implements a deferred frame construction using the defctorid as the location. This is done for all frames that have their frame_alloc node in an internal proc, function or trigger.

Your history entry related to this states:

** 011 CA  20131115          Static frames defined in an internal entry or trigger must be 
**                           instantiated when the internal entry/trigger executes.

I wrote a testcase that tries to leverage this "new frame created with every invocation" behavior. Here is what I came up with:

def var num as int.

def frame f-ext-proc num.
display num with frame f-ext-proc title "Ext Proc".

procedure int-proc:
   def frame f-int-proc num.
   display num with frame f-int-proc title "Int Proc".
end.

function func returns int ():
   def frame f-func num.
   display num with frame f-func title "Func".
   return num.
end.

on "f6" of num in frame f-ext-proc
do:
   def frame f-trig num.
   display num with frame f-trig title "Trigger".
end.

on "f7" of num in frame f-ext-proc
do:
   run int-proc.
end.

on "f8" of num in frame f-ext-proc
do:
   func().
end.

enable all with frame f-ext-proc.
wait-for go of frame f-ext-proc.

/* results if you type f6, f7, f8, f6, f7, f8 */

/*
┌─Ext Proc─┐
│       num│
│──────────│
│         0│
└──────────┘
┌─Trigger──┐
│       num│
│──────────│
│         0│
└──────────┘
┌─Int Proc─┐
│       num│
│──────────│
│         0│
└──────────┘
┌───Func───┐
│       num│
│──────────│
│         0│
└──────────┘
┌─Trigger──┐
│       num│
│──────────│
│         0│
└──────────┘
┌─Int Proc─┐
│       num│
│──────────│
│         0│
└──────────┘
┌───Func───┐
│       num│
│──────────│
│         0│
└──────────┘
*/

Question 1: Does this testcase (the resulting multiple instance display of the same frame) properly capture the behavior you were trying to implement?

The conversion results use the default initializer like this:

   DeferredFrameInstantiationForIntProcFuncAndTriggerBlocksFExtProc fExtProcFrame = GenericFrame.createFrame(DeferredFrameInstantiationForIntProcFuncAndTriggerBlocksFExtProc.class, "f-ext-proc");

   ...

   DeferredFrameInstantiationForIntProcFuncAndTriggerBlocksIntProcFIntProc intProcFIntProcFrame = null;

   DeferredFrameInstantiationForIntProcFuncAndTriggerBlocksFuncFFunc funcFFuncFrame = null;

   DeferredFrameInstantiationForIntProcFuncAndTriggerBlocksTrigger1FTrig trigger1FTrigFrame = null;

   /**
    * External procedure (converted to Java from the 4GL source code
    * in deferred_frame_instantiation_for_int_proc_func_and_trigger_blocks.p).
    */
   public void execute()
   {
      externalProcedure(new Block()
      {
         public void body()
         {
            ...

            fExtProcFrame.display(elementList0);

            EventList list0 = new EventList();
            list0.addEvent("f6", fExtProcFrame.widgetNum());
            registerTrigger(list0, TriggerBlock0.class, DeferredFrameInstantiationForIntProcFuncAndTriggerBlocks.this);

            ...
         }
      });
   }

   public void intProc()
   {
      internalProcedure(new Block()
      {
         {
            intProcFIntProcFrame = GenericFrame.createFrame(DeferredFrameInstantiationForIntProcFuncAndTriggerBlocksIntProcFIntProc.class, "int-proc-f-int-proc");
         }
         public void body()
         {
            ...

            intProcFIntProcFrame.display(elementList1);
         }
      });
   }

   public integer func()
   {
      return function("func", integer.class, new Block()
      {
         {
            funcFFuncFrame = GenericFrame.createFrame(DeferredFrameInstantiationForIntProcFuncAndTriggerBlocksFuncFFunc.class, "func-f-func");
         }
         public void body()
         {
            ...

            funcFFuncFrame.display(elementList2);

            returnNormal(num);
         }
      });
   }

   public class TriggerBlock0
   extends Trigger
   {
      {
         trigger1FTrigFrame = GenericFrame.createFrame(DeferredFrameInstantiationForIntProcFuncAndTriggerBlocksTrigger1FTrig.class, "trigger1-f-trig");
      }
      public void body()
      {
         ...

         trigger1FTrigFrame.display(elementList3);
      }

Question 2: Is the reason that we are using the default initializer instead of the Block.init() method that the calls to createFrame(), importSharedFrame() and createSharedFrame() need to happen before the BlockManager calls TransactionManager.pushScope()?

If my understanding is correct, I plan to implement something like this as a solution:

   DeferredFrameInstantiationForIntProcFuncAndTriggerBlocksFExtProc fExtProcFrame = GenericFrame.createFrame(DeferredFrameInstantiationForIntProcFuncAndTriggerBlocksFExtProc.class, "f-ext-proc");

   ...

   DeferredFrameInstantiationForIntProcFuncAndTriggerBlocksIntProcFIntProc intProcFIntProcFrame = null;

   DeferredFrameInstantiationForIntProcFuncAndTriggerBlocksFuncFFunc funcFFuncFrame = null;

   DeferredFrameInstantiationForIntProcFuncAndTriggerBlocksTrigger1FTrig trigger1FTrigFrame = null;

   /**
    * External procedure (converted to Java from the 4GL source code
    * in deferred_frame_instantiation_for_int_proc_func_and_trigger_blocks.p).
    */
   public void execute()
   {
      externalProcedure(new Block()
      {
         public void body()
         {
            ...

            fExtProcFrame.display(elementList0);

            EventList list0 = new EventList();
            list0.addEvent("f6", fExtProcFrame.widgetNum());
            registerTrigger(list0, TriggerBlock0.class, DeferredFrameInstantiationForIntProcFuncAndTriggerBlocks.this);

            ...
         }
      });
   }

   public void intProc()
   {
      intProcFIntProcFrame = GenericFrame.createFrame(DeferredFrameInstantiationForIntProcFuncAndTriggerBlocksIntProcFIntProc.class, "int-proc-f-int-proc");

      internalProcedure(new Block()
      {
         public void body()
         {
            ...

            intProcFIntProcFrame.display(elementList1);
         }
      });
   }

   public integer func()
   {
      funcFFuncFrame = GenericFrame.createFrame(DeferredFrameInstantiationForIntProcFuncAndTriggerBlocksFuncFFunc.class, "func-f-func");

      return function("func", integer.class, new Block()
      {
         public void body()
         {
            ...

            funcFFuncFrame.display(elementList2);

            returnNormal(num);
         }
      });
   }

   public class TriggerBlock0
   extends Trigger
   {
      public void preScope()
      {
         trigger1FTrigFrame = GenericFrame.createFrame(DeferredFrameInstantiationForIntProcFuncAndTriggerBlocksTrigger1FTrig.class, "trigger1-f-trig");
      }

      public void body()
      {
         ...

         trigger1FTrigFrame.display(elementList3);
      }

The idea is to place the create factory call into the code just above the BlockManager invocation, which should be logically the same execution state and order as the default initializer that will be executed during new Block() construction (which occurs before the BlockManager invocation).

As you can see, this is a problem for triggers, since the business logic does not instantiate those. The UI code does the instantiation on a delegated basis (this is needed for 4GL compatibility). The idea is to extend the Trigger class with a new method that is optionally emitted. When I move triggers to lambdas, I'll have to migrate that to its own lambda... but that is a problem for another day.

Question 3: Do you see any problems with my approach?

Question 4: Do you see any better way to do this?

As an alternative, I could add the preScope() to Block and use a consistent approach for all block types. This has an added benefit of being more explicit about the dependencies for that code. That is probably the right way to go. As a down side, it means more lambda work later.

Greg Shah wrote:

Constantin:
Question 1: Does this testcase (the resulting multiple instance display of the same frame) properly capture the behavior you were trying to implement?

Correct. Another way to prove that each frame (defined in a trigger/procedure/function) is a new frame is to do a message frame f-int-proc:handle. after the display - a different handle ID will be shown for each invocation.

Question 2: Is the reason that we are using the default initializer instead of the Block.init() method that the calls to createFrame(), importSharedFrame() and createSharedFrame() need to happen before the BlockManager calls TransactionManager.pushScope()?

Correct.

Question 3: Do you see any problems with my approach?

For internal procedures/functions, the approach is OK. But why can't we do the same for triggers, too? Before they are invoked, a new instance is created (see TriggerDefinition.getTrigger), so it will be OK to initialize it in the trigger's default c'tor, no longer needing a preScope method. Or this contradicts the goal of removing the defctorid usage?

Question 4: Do you see any better way to do this?

For internal procedures/functions, no.

And as far as triggers go, we may need to emit two lambdas, one to execute any deferred initialization code (like frame creation), and one for the trigger's body.

Or this contradicts the goal of removing the defctorid usage?

Yes, that's right. I am trying to get rid of all default initializer usage because when we switch Block usage to lambdas, there cannot be a default initializer.

And as far as triggers go, we may I to emit two lambdas, one to execute any deferred initialization code (like frame creation), and one for the trigger's body.

We will most likely have a lambda for each backing method used in the current TriggerBlock anon instance approach (pre, init, body...). I will setup the constructors to accept different combinations of these lambdas and will probably differentiate them using a no-parm, void returning functional interface that is used as a marker (instead of Runnable). This way we can cast the lambdas to the "correct" type and the proper constructor will be chosen. Otherwise we have to pass extra nulls all over the place to always have placeholders...

But I have quite a bit of additional work to get rid of all non-method usage in the Block code, before I can then work on emitting the methods as lambdas.

Rebased task branch 1735e from trunk 11088. The latest revision is now 11102.

Task branch 1735e revision 11103 has changes that replace all defctorid usage (and the corresponding default constructor for anonymous Block instances and trigger inner class defs) with a new preid annotation and a pre() method that gets invoked by the BlockManager before the TransactionManager.pushScope() call.

Diff output as an example:

*** old/src/com/goldencode/testcases/DeferredFrameInstantiationForIntProcFuncAndTriggerBlocks.java    2016-08-25 16:49:00.000000000 -0400
--- src/com/goldencode/testcases/DeferredFrameInstantiationForIntProcFuncAndTriggerBlocks.java    2016-08-30 14:49:43.798453044 -0400
***************
*** 65,73 ****
--- 65,75 ----
     {
        internalProcedure(new Block()
        {
+          public void pre()
           {
              intProcFIntProcFrame = GenericFrame.createFrame(DeferredFrameInstantiationForIntProcFuncAndTriggerBlocksIntProcFIntProc.class, "int-proc-f-int-proc");
           }
+ 
           public void body()
           {
              intProcFIntProcFrame.openScope();
***************
*** 86,94 ****
--- 88,98 ----
     {
        return function("func", integer.class, new Block()
        {
+          public void pre()
           {
              funcFFuncFrame = GenericFrame.createFrame(DeferredFrameInstantiationForIntProcFuncAndTriggerBlocksFuncFFunc.class, "func-f-func");
           }
+ 
           public void body()
           {
              funcFFuncFrame.openScope();
***************
*** 108,116 ****
--- 112,122 ----
     public class TriggerBlock0
     extends Trigger
     {
+       public void pre()
        {
           trigger1FTrigFrame = GenericFrame.createFrame(DeferredFrameInstantiationForIntProcFuncAndTriggerBlocksTrigger1FTrig.class, "trigger1-f-trig");
        }
+ 
        public void body()
        {
           trigger1FTrigFrame.openScope();

There are many changes to the rules but the result in the generated code is surprisingly close to the old version.

Rebased task branch 1735e from trunk 11093. The latest revision is now 11113.

Created task branch 1735f from P2J trunk 11093.

Task branch 1735e revisions 11118 and 11119 have been merged into 1735f. These changed represent fixes for rectangle-specific attribute support (conversion and server-side runtime). The changes were regression tested as part of 1735e and passed MAJIC conversion and runtime testing, conversion testing for the customer's Server and GUI projects and ETF testing. This change is needed to properly convert the latest level of the customer's GUI code.

Task branch 1735f was merged to trunk as revision 11094 and then was archived.

I uncommitted the 11118 and 11119 revisions (rectangle-specific attributes) from 1735e. I then rebased 1735e from trunk 11094 (which added the rectangle-specific attributes support back in). The latest revision is now 11118.

The resulting branch is equivalent to the previous 1735e rev 11119 which passed most testing.

Rebased 1735e from trunk 11095. The latest revision is now 11123.

Revision 11120 fixes the SharedVariableManager to properly support case-insensitive shared resources. This bug only can be seen when the legacy name is being used to share and import the resource. For this reason, it did not affect temp-tables which use a non-legacy name (created at conversion time). Shared variables were affected and are now fixed. I haven't checked which others were affected.

Revisions 11121 through 11123 remove the "always promote queries" and revise how they are initialized so that the local variables can be constructed as effectively final but the initialization can be deferred until the next scope is pushed. These changes fix the regressions in the customer GUI app, documented in note 46 and #3213. More details will be posted shortly.

MAJIC regression testing passed and so did the ETF testing. The regression only manifests in the customer GUI app (see #3213). A temp-table named ttLscasubac is "new shared" by sc/Lscensb0.java. It is imported "shared" by sc/Lscenfn0.java.

I instrumented the SVM with some very verbose logging (of all Scopable notifications + specific outputs when ttLscasubac is added and removed from the SVM). I captured (in the server log) the output in both trunk and in 1735e rev 11119. I've placed the logs and instrumented SVM in your home dir on devsrv01 (see logs_from_1735e_bug_diagnosis.zip and svm_logging_version.zip).

The recreation is simple:

• start the swing client
• select the 454 testcase
• in 1735e, by the time the User Defined Alerts window appears, the scope that contains the ttLscasubac table has already been removed
• in trunk, it is never removed
• at that point subsequent access rightly fails in 1735e

ges@ges:~/projects/<customer>/gui/run/server/logs$ cat trunk_log1 | grep "ttLscasubac" | wc -l
2749
ges@ges:~/projects/<customer>/gui/run/server/logs$ cat trunk_log1 | grep "ttLscasubac" | uniq
key = ttLscasubac, value = com.goldencode.p2j.persist.$__Proxy47@6c716fa8
--- --- --- --- --- --- --- --- --- ttLscasubac=com.goldencode.p2j.persist.$__Proxy47@6c716fa8

ges@ges:~/projects/<customer>/gui/run/server/logs$ cat 1735e_11119_log2 | grep "ttLscasubac" | wc -l
1947
ges@ges:~/projects/<customer>/gui/run/server/logs$ cat 1735e_11119_log2 | grep "ttLscasubac" | uniq
key = ttLscasubac, value = com.goldencode.p2j.persist.$__Proxy47@6a880285
--- --- --- --- --- --- --- --- --- ttLscasubac=com.goldencode.p2j.persist.$__Proxy47@6a880285
HERE HERE HERE! ttLscasubac SCOPE REMOVED!
[09/13/2016 11:30:54 EDT] (ErrorManager:SEVERE) {00000001:00000012:<customer>} Unable to locate shared temp-table or work-table definition for table ttLscasubac in procedure lscenfn0.p. (1429)

The removal in 1735e occurs at this point:

        at com.goldencode.p2j.util.SharedVariableManager.scopeDeleted(SharedVariableManager.java:1487)
        at com.goldencode.p2j.util.TransactionManager.processScopeNotifications(TransactionManager.java:5296)
        at com.goldencode.p2j.util.TransactionManager.popScope(TransactionManager.java:2015)
        at com.goldencode.p2j.util.BlockManager.topLevelBlock(BlockManager.java:6804)
        at com.goldencode.p2j.util.BlockManager.externalProcedure(BlockManager.java:294)
        at com.goldencode.p2j.util.BlockManager.externalProcedure(BlockManager.java:268)
        at com.something.gui.common.Amndsfn0.execute(Amndsfn0.java:27)
        at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
        at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
        at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
        at java.lang.reflect.Method.invoke(Method.java:498)
        at com.goldencode.p2j.util.ControlFlowOps$InternalEntryCaller.invokeImpl(ControlFlowOps.java:5395)
        at com.goldencode.p2j.util.ControlFlowOps$InternalEntryCaller.invoke(ControlFlowOps.java:5373)
        at com.goldencode.p2j.util.ControlFlowOps.invokeImpl(ControlFlowOps.java:4434)
        at com.goldencode.p2j.util.ControlFlowOps.invoke(ControlFlowOps.java:3415)
        at com.goldencode.p2j.util.ControlFlowOps.invokeImpl(ControlFlowOps.java:4111)
        at com.goldencode.p2j.util.ControlFlowOps.invokeImpl(ControlFlowOps.java:4040)
        at com.goldencode.p2j.util.ControlFlowOps.invokeWithMode(ControlFlowOps.java:349)
        at com.goldencode.p2j.util.ControlFlowOps.invokeWithMode(ControlFlowOps.java:331)
        at com.something.gui.common.Aguitst0.lambda$null$14(Aguitst0.java:625)
        at com.goldencode.p2j.util.Block.body(Block.java:370)
        at com.goldencode.p2j.util.BlockManager.processBody(BlockManager.java:6936)
        at com.goldencode.p2j.util.BlockManager.doBlockWorker(BlockManager.java:7877)
        at com.goldencode.p2j.util.BlockManager.doBlock(BlockManager.java:718)
        at com.something.gui.common.Aguitst0.lambda$launchGuiscreen$15(Aguitst0.java:610)
        at com.goldencode.p2j.util.Block.body(Block.java:370)
        at com.goldencode.p2j.util.BlockManager.processBody(BlockManager.java:6936)
        at com.goldencode.p2j.util.BlockManager.topLevelBlock(BlockManager.java:6727)
        at com.goldencode.p2j.util.BlockManager.internalProcedure(BlockManager.java:320)
        at com.goldencode.p2j.util.BlockManager.internalProcedure(BlockManager.java:306)
        at com.something.gui.common.Aguitst0.launchGuiscreen(Aguitst0.java:583)
...

The strange thing is that common.Amndsfn0 appears 5586 times in the trunk log. Based on my lambda changes, there should be exactly half the number of appearances in the 1735e log. Instead, we see 4217 entries and the last entry is the thing that causes the removal of the scope associated with the ttLscasubac temp-table (even though that scope was created by a completely different program).

I guess is that somehow the resource deletion processing is mismatched with my changes. I did have to make a change to bypass the execution of ProcedureManager.findRootEnclosingInstance() which uses reflection and relies upon the inner class structure of the Block anon inner class mode. It is only used now for non-lambda mode Block instances and with the TriggerDefinition helper class. I think that is OK, but something is wrong somewhere.

At this point, in addition to debugging sessions where I tried to work backwards from the failure, I got into a discussion with Constantin. He was able to duplicate the same problem, so it was not something environmental. He then posted this answer:

It finally hit me what is wrong with 1735e. You are promoting query variables as instance fields, instead of keeping them with the query loop. This gets problematic when the same persistent procedure has an internal proc/function invoked, and this appears one or more times in the stack. See this example:

def temp-table tt1 field f1 as int.

create tt1.
tt1.f1 = 10.
create tt1.
tt1.f1 = 20.

procedure proc0:
def input param i as int.

   for each tt1 where tt1.f1 > i:
      message tt1.f1.

      run proc0(tt1.f1 + 1).
   end.
end.

run proc0(9).

It should print 10,20,20. But P2J stops at 10,20. Why: on second invocation of proc0, the query0 (as is an instance field) is initiated again, and the previous query (from the stack) gets lost. When proc0 2nd invocation returns, it doesn't see the same query...

and this:

Further thinking, no procedure-specific resource can be emitted as an instance field for the external program, for the same reason as the for-each query.

From what I can see, we have problems only with frames and defined queries:

procedure proc0.
   def input param l as log.
   def buffer b for book.
   form l with frame f1.
   def query q for b.
   message l buffer b:handle frame f1:handle query q:handle.

   if l then run proc0(false).

   message l buffer b:handle frame f1:handle query q:handle.
end.

run proc0(true).

From this test, P2J emits as instance fields these:

   ResTestProc0F1 proc0F1Frame = null;

   QueryWrapper query0 = null;

They should be local to the internal procedure.

For resources (including queries and frames) which the 4GL scopes to a callable top-level block (functions, triggers, internal procedures), recursion is a problem if we promote those resources to be instance members of the containing class (which is equivalent to scoping for the external procedure in the 4GL).

Actually, we have existing problems in trunk with this (for both frames and queries). But in 1735e, I created a new problem because I was promoting all queries to be instance members. MAJIC and ETF testing did not use this "feature" in any case where they did recurse. The GUI app with its heavy ADM/ADM2 implementation was broken by this new promotion of all queries.

The problem to be solved here is that the query hierarchy was designed to configure most instances using parameters to the constructor. Some of the initialization logic that was being executed at that point has dependencies on running while inside the scope to which it is being attached. This means that we emitted these constructors in the init() method of the block inner class. This "split" definition approach worked fine in an inner class because what the compiler detects as a re-assignment of the query member is legal. In a lambda approach it makes them not effectively final and thus ineligible for reference from within a lambda. But we know that the initialization logic must be done inside the Init lambda.

In revisions 11121 through 11123, I have reworked the query hierarchy and the conversion of queries. We now construct the queries separately from where we instantiate them. We construct at the local instance variable location but we initialize in the Init lambda. Old approach:

            forEachWhile("loopLabel4", () -> new logical(false), new Block()
            {
               AdaptiveQuery query3 = null;

               public void init()
               {
                  query3 = new AdaptiveQuery(tt, (String) null, null, "tt.num1 asc");
               }

               public void enter()
               {
                  query3.next();
               }

               public void body()
               {
                  ...
               }

            });

New approach:

         AdaptiveQuery query3 = new AdaptiveQuery();

         forEachWhile("loopLabel4", () -> new logical(false), new Block((Init) () -> 
         {
            query3.initialize(tt, ((String) null), null, "tt.num1 asc");
         }, 
         (Enter) () -> 
         {
            query3.next();
         }, 
         (Body) () -> 
         {
         }));

All queries (except for FindQuery and AdaptiveFind) are now defined and instantiated at the same location. These cases always use a default constructor. The initialization logic is executed in the Init lambda using one of the initialize() method variants which match one to one with the overloaded constructors that used to exist (but which no longer exist).

You MUST always call initialize() after each default constructor usage and before any usage of the class (including calls to things like CompoundQuery.addComponent(). initialize() contains all the logic in the old constructors. We also return this from initialize() so that it can be chained with the constructor call. This is used for QueryWrapper.assign() and CompoundQuery.addComponent() idioms. Old way:

   final QueryWrapper query0 = new QueryWrapper("q", false);
   ...

      externalProcedure(new Block()
      {
         ...

         public void body()
         {
            query0.addBuffer(person, true);
            query0.addBuffer(persAddr, true);
            query0.addBuffer(address, true);
            f3Frame.openScope();
            RecordBuffer.openScope(person, persAddr, address);
            query0.assign(new CompoundQuery(true, false));
            query0.addComponent(new AdaptiveQuery(person, "?", null, "person.lastName asc, person.firstName asc, person.middleInit asc", "WHOLE-INDEX,name", new Object[]
            {
               (P2JQuery.Parameter) () -> isEqual(leading(), 2)
            }, LockType.NONE));
            query0.addComponent(new AdaptiveQuery(persAddr, "? = persAddr.empNum and upper(persAddr.linkType) = ? and ? and ?", null, "persAddr.linkType asc", "link-type", new Object[]
            {
               new FieldReference(person, "empNum"),
               toUpperCase(target),
               (P2JQuery.Parameter) () -> notUnknown(leading()),
               (P2JQuery.Parameter) () -> notUnknown(trimLeading())
            }, LockType.NONE));
            query0.addComponent(new AdaptiveQuery(address, "? = address.addrId and ?", null, "address.addrId asc", "addr-id", new Object[]
            {
               new FieldReference(persAddr, "addrId"),
               (P2JQuery.Parameter) () -> isEqual(leading(), 2)
            }, LockType.NONE));
            query0.open();
            query0.first();

New approach:

   final QueryWrapper query0 = new QueryWrapper("q", false);

   ...

      externalProcedure(ClientWherePromptQueryFor1.this, new Block((Init) () -> 
      {
         ClientWherePromptQueryFor1.this.target = target;
      }, 
      (Body) () -> 
      {
         query0.addBuffer(person, true);
         query0.addBuffer(persAddr, true);
         query0.addBuffer(address, true);
         f3Frame.openScope();
         RecordBuffer.openScope(person, persAddr, address);
         query0.assign(new CompoundQuery().initialize(true, false));
         query0.addComponent(new AdaptiveQuery().initialize(person, "?", null, "person.lastName asc, person.firstName asc, person.middleInit asc", "WHOLE-INDEX,name", new Object[]
         {
            (P2JQuery.Parameter) () -> isEqual(leading(), 2)
         }, LockType.NONE));
         query0.addComponent(new AdaptiveQuery().initialize(persAddr, "? = persAddr.empNum and upper(persAddr.linkType) = ? and ? and ?", null, "persAddr.linkType asc", "link-type", new Object[]
         {
            new FieldReference(person, "empNum"),
            toUpperCase(target),
            (P2JQuery.Parameter) () -> notUnknown(leading()),
            (P2JQuery.Parameter) () -> notUnknown(trimLeading())
         }, LockType.NONE));
         query0.addComponent(new AdaptiveQuery().initialize(address, "? = address.addrId and ?", null, "address.addrId asc", "addr-id", new Object[]
         {
            new FieldReference(persAddr, "addrId"),
            (P2JQuery.Parameter) () -> isEqual(leading(), 2)
         }, LockType.NONE));
         query0.open();
         query0.first();

Notice how the chaining is used in both the assign() and addComponent() cases.

FindQuery and AdaptiveFind are exceptions to this new rule. These cases always emit in-line in the code because they are not associated with a block header or loop header. For this reason, they can never be promoted and they cannot break recursion behavior. For these cases, the old constructor idiom remains the same.

This does fix the issue found in customer GUI app testing (note 46 and #3213). I have created a separate task to resolve the recursion issues that already exist in trunk, see #3183.

Revision 11123 has passed the following re-testing:

• MAJIC conversion
• Customer GUI app conversion and runtime scenario testing

MAJIC runtime testing is in process, as is the customer Server project conversion and ETF testing. If these pass I will be merging 1735e to trunk.

MAJIC did require some manual srcnew/ edits in order to match the latest 1735e revision 11123 approach. See #3180 for details.

Rebased from trunk revision 11096. The latest revision is now 11125.

A regression was found in MAJIC runtime testing. That NPE was a conversion error and was resolved in revision 11125. Conversion and runtime regression testing is restarting.

MAJIC conversion testing passed but runtime testing was broken (GSO 67 and GSO 133). Revision 11126 is meant to resolve this issue.

Rebased 1735e from trunk revision 11097. The latest revision is now 11127.

One more conversion fix was required to resolve a remaining problem with GSO 67 and GSO 133. This was made in rev 11128. After that, these tests now pass. GSO 156 failed but passed when run manually. There were some dc slot tests that failed but which were expected to fail at the time of day when they were run and they had passed recently. MAJIC regression testing has passed.

Task branch 1735e was rebased from trunk revision 11098. The latest revision is now 11129.

This version has been tested with the customer GUI app scenarios and is working properly. There is no reason to repeat the MAJIC testing. At this time, we are only needing to get the customer Server project conversion and ETF test results to know if it is safe to merge to trunk.

Due to the very large number of updates being integrated into the trunk today and tomorrow, the plan is to let those all merge first and then to do a follow-up round of testing before 1735e merges to trunk.

Task branch 1735e was rebased from trunk revision 11101. The latest revision is now 11132.

Task branch 1735e was rebased from trunk revision 11102. The latest revision is now 11133.

MAJIC regression testing has passed for 1735e revision 11133.

Task branch 1735e was rebased from trunk revision 11105. The latest revision is now 11136.

Merged 1735e to trunk as revision 11106 and sent notification email. Archived 1735e.

Created new task branch 1735g from trunk revision 11106.

Created task branch 1735h from trunk 11106. There is a regression in RuntimeJastInterpreter exposed by the ETF testing. Igor came up with a fix and I have applied it to 1735h.

Eric is pushing this to the staging ETF pipeline and if that passes testing we will merge this to trunk.

Igor's testing as posted in #3109-505 shows 1735h is safe.

1735h has been merged to trunk as revision 11107 and 1735h was archived.

Task branch 1735g rebased from trunk revision 11107. The latest revision is now 11108.

Igor has just asked a good question:

Do you know how to decipher lambda expressions' references in the call stack like this:
com.something.server.common.Aapisrch$$Lambda$332.body(),
com.something.server.common.Aapisrch.lambda$sPapiSrchCopyStatTtToResTt$18(handle, handle, handle, handle, handle, logical, integer) or
com.something.server.common.Aapisrch.lambda$null$17(handle, handle, handle, handle, integer, handle).
I've tried to find the information in the Internet but found nothing useful.

While I can't give you a complete answer, here are some things to know:

When a lambda is encountered at runtime, there is a multi-phase process by which the JVM prepares and executes the call site. The preparation is managed by a "meta-factory". The meta-factory is doing dynamic/runtime linkage of the call site to the functional interface and must also capture the references to the containing scopes. As part of this process, the runtime implementation of the call site is re-written. In the OpenJDK case, this meta-factory code can be found in
java/lang/invoke/*Metafactory.java:

java/lang/invoke/LambdaMetafactory.java (factory for meta-factories)
java/lang/invoke/AbstractValidatingLambdaMetafactory.java (abstract base class for a meta-factory)
java/lang/invoke/InnerClassLambdaMetafactory.java (concrete implementation of the meta-factory)

The InnerClassLambdaMetafactory creates an inner class structure for the lambdas, but it does so at runtime. This is the code that leads to the funny naming on the call stack.

The "$$Lambda" is just some hard coded text in InnerClassLambdaMetafactory, used as a separator. I think the "$332" is
just an ordinal specifying the which lambda in the compilation unit is being processed. The ".body()" is the method of the functional interface being executed, which is about the only useful thing here. I think you will have to leverage the line numbers to get anything useful from the stack trace.

In the second case, the "sPapiSrchCopyStatTtToResTt" is a reference to a method of Aapisrch.java. I suspect that the "(handle, handle, handle, handle, handle, logical, integer)" is a description of the captured
local variable references from the containing scope: handle ihResultTtbuf, handle ihStaticTtbuf, integer iiParentApiSeqNo, handle vhQuery, logical vlWorked, handle vhParentApiSeqNoField, handle vhApiSeqNoField. The difference in order is probably due to the lambda capturing being done in order of reference rather than the lexical order of appearance of the definitions. This last bit is pure supposition on my part, I haven't analyzed the meta-factory code enough to know for sure.

Task branch 1735g rebased from trunk revision 11110. The latest revision is now 11114.

Task branch 1735g rebased from trunk revision 11111. The latest revision is now 11117.

1735g revision 11116 passed MAJIC conversion and runtime regression testing.

Eric reports that 1735g rev 11117 passed customer Server project conversion and all ETF testing using the staging pipeline.

1735g rev 11120 passed MAJIC conversion and runtime regression testing.

1735g rev 11121 passed MAJIC conversion testing and requires no runtime testing as it is conversion only and doesn't change MAJIC from 11120.

I am reconverting the customer GUI code with 11121 right now and assuming it passes (11120 did), I will manually regression test it. I expect it to work properly but won't know until sometime tomorrow.

Eric: Please push this to the staging pipeline for both conversion and ETF testing.

I will rebase this branch tomorrow morning. I don't plan to retest after rebase as there are no conflicts.

1735g rev 11121 has passed customer GUI code conversion and runtime testing.

If the customer Server code conversion and ETF testing pass, then I will merge this branch to trunk.

Task branch 1735g rebased from trunk revision 11113. The latest revision is now 11123.

Greg Shah wrote:

Eric: Please push this to the staging pipeline for both conversion and ETF testing.

1735g/11121 passed conversion and runtime ETF testing in the staging pipeline.

Task branch 1735g rebased from trunk revision 11115. The latest revision is now 11125.

Task branch 1735g has been merged to trunk as revision 11116 (refs #1735). This revision continues to shift the use of inner blocks into a lambda implementation.

The following describes the changes:

1. Additional blank lines will be seen before and after IF/THEN/ELSE, before and after CASE statements, before and after trigger registration and before WAIT-FOR.

2. The Block() class now allows its default constructor to be used, which is equivalent to a completely empty block (no lambdas passed means that none of the methods have an implementation). This is useful in scenarios seen in item 3 below.

3. Empty lambdas that were previously emitted, are now removed. This could happen for Body lambdas and also in the case where there was a completely empty block (previously it would emit with all 5 empty lambdas because the deletion code was only processed in an ascent rule and in a completely empty block there is no ascent because there are no children nodes).

So where you might see this previously:

         forEachWhile("loopLabel4", () -> new logical(false), new Block((Init) () ->
         {
            query3.initialize(tt, ((String) null), null, "tt.num1 asc");
         },
         (Enter) () ->
         {
             query3.next();
         },
         (Body) () ->
         {
         }));

Now you will see this:

         forEachWhile("loopLabel4", () -> new logical(false), new Block((Init) () ->
         {
            query3.initialize(tt, ((String) null), null, "tt.num1 asc");
         },
         (Enter) () ->
         {
            query3.next();
         }));

And something like this (an internal procedure that has no implementation in the 4GL) looked like this:

       internalProcedure(new Block((Pre) () ->
       {
       },
       (Init) () ->
       {
       },
       (Enter) () ->
       {
       },
       (Body) () ->
       {
       },
       (Fini) () ->
       {
       }));

And now it looks like this:

       internalProcedure(new Block());

4. The common case for EventList is now implemented as an inline constructor. The idea is that 99% of all usage of EventList is for a single OF clause. In these cases, the old way looked like this:

         EventList list0 = new EventList();
         list0.addEvent("x", fFrame.widgetCh());
         registerTrigger(list0, TriggerBlock0.class, TriggerLocalVar.this);

Constructors have been added to match all forms of the addEvent() method and the conversion now emits these parameters into the constructor when there is only a single OF clause (event_list OF widget_list). The new approach looks like this:

        registerTrigger(new EventList("x", fFrame.widgetCh()), TriggerBlock0.class, TriggerLocalVar.this);

For the more rare scenario where there are multiple OF clauses, the old separate EventList approach will still be used.

5. The GO-ON clause is now implemented as its own convenience sub-class of EventList, which enables the most common GO-ON cases to be implemented as an inline constructor. The old way looked like this:

          EventList list3 = new EventList();
          list3.addGoOn("enter");
          fFrame.update(elementList5, list3);

The new way:

          fFrame.update(elementList5, new GoOn("enter"));

6. UI triggers that do not have local variable definitions or local buffer definitions are now emitted as lambdas. The old way looked like this:

         EventList list0 = new EventList();
         list0.addEvent("f6", fExtProcFrame.widgetNum());
         registerTrigger(list0, TriggerBlock0.class, DeferredFrameInstantiationForIntProcFuncAndTriggerBlocks.this);
         EventList list1 = new EventList();
         list1.addEvent("f7", fExtProcFrame.widgetNum());
         registerTrigger(list1, TriggerBlock1.class, DeferredFrameInstantiationForIntProcFuncAndTriggerBlocks.this);
         ...
      }));
   }
   ...
   public class TriggerBlock0
   extends Trigger
   {
      public void pre()
      {
         trigger1FTrigFrame = GenericFrame.createFrame(DeferredFrameInstantiationForIntProcFuncAndTriggerBlocksTrigger1FTrig.class, "trigger1-f-trig");
      }

      public void body()
      {
         trigger1FTrigFrame.openScope();

         FrameElement[] elementList3 = new FrameElement[]
         {
            new Element(num, trigger1FTrigFrame.widgetNum())
         };

         trigger1FTrigFrame.display(elementList3);
      }
   }

   public class TriggerBlock1
   extends Trigger
   {
      public void body()
      {
         ControlFlowOps.invoke("int-proc");
      }
   }

The new way:

         registerTrigger(new EventList("f6", fExtProcFrame.widgetNum()), DeferredFrameInstantiationForIntProcFuncAndTriggerBlocks.this, new Trigger((Pre) () ->
         {
            trigger1FTrigFrame = GenericFrame.createFrame(DeferredFrameInstantiationForIntProcFuncAndTriggerBlocksTrigger1FTrig.class, "trigger1-f-trig");
         },
         (Body) () ->
         {
            trigger1FTrigFrame.openScope();

            FrameElement[] elementList3 = new FrameElement[]
            {
               new Element(num, trigger1FTrigFrame.widgetNum())
            };

            trigger1FTrigFrame.display(elementList3);
         }));

         registerTrigger(new EventList("f7", fExtProcFrame.widgetNum()), DeferredFrameInstantiationForIntProcFuncAndTriggerBlocks.this, new Trigger((Body) () ->
         {
            ControlFlowOps.invoke("int-proc");
         }));

This will be the way that the vast majority of triggers will emit. The exception is when local resources are scoped to the trigger itself. This can be done for variables and buffers. In these cases, any recursive use of that trigger would possibly break in the lambda approach because the local resources would be a shared copy whereas in the 4GL, each trigger invocation would have its own instance of the variable or buffer. Our previous inner class implementation already handled this behavior properly. For this reason, we detect these cases and then use inner class mode instead of lambdas for the associated trigger. This is a pretty rare case and so most UI triggers will emit using lambdas.

Database triggers always use the inner class mode (they have not yet been migrated to lambdas).

The runtime has been written to be compatible with the prior implementation, so you should not need to reconvert immediately. When you do reconvert, you can expect a very large set of changes in MAJIC and customer GUI code in particular. There are some changes to customer Server code too, but it is less pervasive since the changes are to UI code.

Branch 1735g has been archived.

Created task branch 1735i from trunk rev 11120.

Branch 3330a revision 11186 provides support for complex BY expressions emitted as lambdas.

We sometimes see converted code which causes a fatal javac problem "error: too many parameters". This can happen without any single explicit method call which has more than 255 parameters. The following is intended to explain what is happening.

The first thing to know about the "error: too many parameters" javac failure is that it is related to how many parameters are being passed to a method. Java allows up to 255 parameters to be passed to any method. Since instance methods have an implicit "this" parameter, there is a real limit of 254 parameters for instance methods. static methods can get the full 255 formal parameters.

Unfortunately, javac does not provide any diagnostic data at all for this error. There is no way to enable better data. So we must look at the code to see where the problem lies.

The second thing to know about this problem: it isn't always related to the formal parameter list provided in the source code.

The 4GL is described as a block structured language. It has a huge amount of implicit behavior encoded and reliant upon the block structure of the program, which can be (and often is) heavily nested. This affects transactions, sub-transactions, error processing, implicit retry of blocks, control flow, undo, infinite loop protection for retry, resource scoping, record locking, buffer release... an insane amount of hidden behavior.

In order to implement compatible behavior in FWD, we heavily delegate processing of the converted code to the FWD runtime. This is done by emitting the converted code as "anonymous methods". The common name for this is a "lambda". In Java, there is syntax to define a method which is a "chunk" of code that can be passed around or saved as data. It can be passed through a call chain down to some place in the FWD runtime and then called from that location zero or more times as needed.

Lambdas are special in many ways. One way is that they can directly reference local variables that are final or effectively final. Effectively final means a variable that is initialized/assigned once and then never re-assigned. These variables can live inside the method that contains the lambda(s). So these variables can be truly local data (not members of the enclosing class) and they can be referenced from these anonymous code chunks (methods) within that scope.

Each block in the 4GL creates at least one lambda in the converted code (for the block body itself) and possibly some other lambdas like one for initialization, catch blocks or for a finally block.

Nested blocks are also lambda, but they are nested inside containing lambdas which are ultimately nested inside some named method in the converted business logic class.

Since lambdas are really just an anonymous method, the java compiler (javac) has to do quite a bit of implicit processing to create these methods inside the bytecode (the compiled instructions in the "binary" class file). If the code inside a lambda references a local variable in the containing outer method, the compiler creates an implicit parameter for the lambda and passes that local variable reference along so that the chunk of code will see it and be able to reference it.

These implicit parameters "add up" because the deeply nested lambdas may be referencing local data which must be passed down through each containing lambda as implicit parameters. In other words, deeply nested references to local data get accumulated as implicit parameters so that at the top level of lambdas there can be a large number of parameters even if it is not obvious that this is the case.

Reducing the number of references or splitting the 4GL code into more than 1 method would solve the issue so long as it was no more than 254 local var references for that method.