Details

    • Type: New Feature New Feature
    • Status: In Progress
    • Priority: Major Major
    • Resolution: Unresolved
    • Affects Version/s: 1.4
    • Fix Version/s: 2.4.0-RC1
    • Component/s: core
    • Labels:
      None
    • Environment:
      Target Click 1.5

      Description

      I have been following both OGNL and MVEL (another expression language) development the last couple of months. If we are going to make changes to this here are some things you might find interesting:

      • OGNL 2.7.1 which is included in tapestry 4.1, now sports byte code enhancement. This was added by one of the tapestry authors to make OGNL much faster than its reflection mode. However in my testing 2.7.1 threw exceptions every now and then. So still buggy.
      • MVEL (http://mvel.codehaus.org/) seems like a good replacement for OGNL. It has good docs and is actively developed. They even fixed a bug I logged! Like OGNL, MVEL also runs in two modes, either reflection or byte compiled. Unlike OGNL, MVEL's reflection mode is faster than Click's reflection mode.

      I am no expert on this but according to the article below (from MVEL's author), byte code enhancement have some problems in that generated classes accumulate in java's perm space and will only be removed when their classloader is removed.
      http://artexpressive.blogspot.com/2007/07/mvel-by-numbers-real-story.html

      So MVEL in reflection mode looks like an ideal solution here. The power of OGNL with the performance of Click reflection.

      Table columns can again take advantage of expressions. For example to aggregate a total for the row -> new Column("price * tax");

      1. ASF.LICENSE.NOT.GRANTED--expression-language-performance.rar
        5 kB
        Bob Schellink
      2. ASF.LICENSE.NOT.GRANTED--expression-language-performance.zip
        5 kB
        Bob Schellink
      3. mvel.zip
        1 kB
        Gilberto C Andrade
      4. patch.diff
        2 kB
        Gilberto C Andrade

        Activity

        Hide
        Malcolm Edgar added a comment -

        Agree with the usage of reflection instead of byte code enhancement.

        MVEL has a backport for JDK 1.4 which would be appropriate for Click:

        http://mvel.codehaus.org/About+Java+1.4+Support

        OGNL is used ClickServlet.processPageRequestParams and ClickUtils.copyFormToObject

        Show
        Malcolm Edgar added a comment - Agree with the usage of reflection instead of byte code enhancement. MVEL has a backport for JDK 1.4 which would be appropriate for Click: http://mvel.codehaus.org/About+Java+1.4+Support OGNL is used ClickServlet.processPageRequestParams and ClickUtils.copyFormToObject
        Hide
        Ahmed Mohombe added a comment -

        > * MVEL (http://mvel.codehaus.org/) seems like a good replacement for OGNL.
        > It has good docs and is actively developed. They even fixed a bug I logged!
        > Like OGNL, MVEL also runs in two modes, either reflection or byte compiled.
        > Unlike OGNL, MVEL's reflection mode is faster than Click's reflection mode.
        I see they have different releases for different JDKs (e.g. a distinct one for 1.6).
        How is this performance when using the "wrong" version (e.g. 1.4 release in a 1.6 JDK) ?
        Click can bundle only one of them in the click.jar. It would be a huge disadvantage if
        uses would need to manually "solve" the dependencies based on their JDK (till yet users
        didn't have to bother - just drop in the click.jar and ready).

        > I am no expert on this but according to the article below (from MVEL's author), byte code enhancement have some problems in that generated classes accumulate in java's perm space and will only be removed when their classloader is removed.
        > http://artexpressive.blogspot.com/2007/07/mvel-by-numbers-real-story.html
        Everybody knows that bytecode manipulation is evil . It simply cuts the JIT's possibility
        to do it's smart things as it was designed.

        > So MVEL in reflection mode looks like an ideal solution here.
        I see it like "yet another dependency" and possible source of errors .

        If something goes wrong, I prefer to hunt and fix the problem in the Click code,
        and not in the 1.2MB MVEL sourcecode (much bigger than Click) .

        IMHO performance is very important, but it should be the last step, and IMHO
        we are not at the "last step" with the click development .

        just my 2 cents,

        Ahmed.

        -------------------------------------------------------------------------
        This SF.net email is sponsored by: Microsoft
        Defy all challenges. Microsoft(R) Visual Studio 2008.
        http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/
        _______________________________________________
        Click-development mailing list
        Click-development@lists.sourceforge.net
        https://lists.sourceforge.net/lists/listinfo/click-development

        Show
        Ahmed Mohombe added a comment - > * MVEL ( http://mvel.codehaus.org/ ) seems like a good replacement for OGNL. > It has good docs and is actively developed. They even fixed a bug I logged! > Like OGNL, MVEL also runs in two modes, either reflection or byte compiled. > Unlike OGNL, MVEL's reflection mode is faster than Click's reflection mode. I see they have different releases for different JDKs (e.g. a distinct one for 1.6). How is this performance when using the "wrong" version (e.g. 1.4 release in a 1.6 JDK) ? Click can bundle only one of them in the click.jar. It would be a huge disadvantage if uses would need to manually "solve" the dependencies based on their JDK (till yet users didn't have to bother - just drop in the click.jar and ready). > I am no expert on this but according to the article below (from MVEL's author), byte code enhancement have some problems in that generated classes accumulate in java's perm space and will only be removed when their classloader is removed. > http://artexpressive.blogspot.com/2007/07/mvel-by-numbers-real-story.html Everybody knows that bytecode manipulation is evil . It simply cuts the JIT's possibility to do it's smart things as it was designed. > So MVEL in reflection mode looks like an ideal solution here. I see it like "yet another dependency" and possible source of errors . If something goes wrong, I prefer to hunt and fix the problem in the Click code, and not in the 1.2MB MVEL sourcecode (much bigger than Click) . IMHO performance is very important, but it should be the last step, and IMHO we are not at the "last step" with the click development . just my 2 cents, Ahmed. ------------------------------------------------------------------------- This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2008. http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/ _______________________________________________ Click-development mailing list Click-development@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/click-development
        Hide
        Malcolm Edgar added a comment -

        My thinking that reflection mode only would be used and not byte code enhancement as described by Bob. I agree byte code enhancement is evil, and I don't want it in the Click Framework. Reading through MVEL doco, and their experiences only re-enforces this belief.

        Click could only include the JDK 1.4 version of MVEL, this would be compatible with latter JDK's presumably.

        A tailored MVEL 1.4 jar is about 700KB in size, larger than OGNLs 165 KB.

        I think we should run some performance tests and see how it compares.

        Your 2 cents is always appreciated.

        regards Malcolm Edgar

        Show
        Malcolm Edgar added a comment - My thinking that reflection mode only would be used and not byte code enhancement as described by Bob. I agree byte code enhancement is evil, and I don't want it in the Click Framework. Reading through MVEL doco, and their experiences only re-enforces this belief. Click could only include the JDK 1.4 version of MVEL, this would be compatible with latter JDK's presumably. A tailored MVEL 1.4 jar is about 700KB in size, larger than OGNLs 165 KB. I think we should run some performance tests and see how it compares. Your 2 cents is always appreciated. regards Malcolm Edgar
        Hide
        Bob Schellink added a comment -

        The performance of MVEL is pretty good. I only tested with the MVEL 1.4 version on JDK 1.4.

        If you recall there was a multi threaded test awhile ago for testing reflection vs ognl. I ran the same test with the following results:

        MVEL with caching cumulative: 1,239ms
        MVEL with thread caching cumulative: 170ms

        Reflection with caching cumulative: 2,010ms
        Reflection with thread caching cumulative: 174ms

        So pretty good performance from both approaches. The reason I think MVEL is interesting is we gain expression language. Click reflection can only navigate paths. MVEL can do this:

        new Column("firstname + ' ' + lastname"); // Handy when you want to display the persons fullname. Currently the value object will have to expose a getName method

        new Column("price1 + price2 + price3 * tax");

        Since OGNL is only used in two places, perhaps we can make it easier to plug in different expression languages.

        We can expose for example ClickServlet#createPropertyUtils method and those who want can create a subclass and use MVEL instead of reflection or ognl.

        Show
        Bob Schellink added a comment - The performance of MVEL is pretty good. I only tested with the MVEL 1.4 version on JDK 1.4. If you recall there was a multi threaded test awhile ago for testing reflection vs ognl. I ran the same test with the following results: MVEL with caching cumulative: 1,239ms MVEL with thread caching cumulative: 170ms Reflection with caching cumulative: 2,010ms Reflection with thread caching cumulative: 174ms So pretty good performance from both approaches. The reason I think MVEL is interesting is we gain expression language. Click reflection can only navigate paths. MVEL can do this: new Column("firstname + ' ' + lastname"); // Handy when you want to display the persons fullname. Currently the value object will have to expose a getName method new Column("price1 + price2 + price3 * tax"); Since OGNL is only used in two places, perhaps we can make it easier to plug in different expression languages. We can expose for example ClickServlet#createPropertyUtils method and those who want can create a subclass and use MVEL instead of reflection or ognl.
        Hide
        Malcolm Edgar added a comment -

        Thanks Bob for the analysis, with regard to the performance numbers what is the difference between caching cumulative and thread caching cumulative?

        Show
        Malcolm Edgar added a comment - Thanks Bob for the analysis, with regard to the performance numbers what is the difference between caching cumulative and thread caching cumulative?
        Hide
        Bob Schellink added a comment -

        If you recall the test ran 20 threads for 1000 iterations. "Caching cumulative" means there is a global cache where compiled expressions are stored. Because the cache is global we needed to synchronize on the cache.

        "Thread caching cumulative" means there is a cache per thread and no synchronization is needed.

        Basically only "Thread caching cumulative" matters since we do not synchronize in Click. Below are results of the test with JDK 1.4, 1.5 and 1.6.

        I have also added tests for setting the value of beans.

        Windows XP, Dual core machine.
        Note the results below are after 2 warmup runs.

        Sun JDK 1.4
        OGNL getter cumulative time: 1,970ms -> (PropertyUtils.getValueOgnl)
        OGNL setter cumulative time: 9,105ms -> (PropertyUtils.setValueOgnl)

        MVEL getter cumulative time: 153ms
        MVEL setter cumulative time: 2,390ms

        Reflection getter cumulative time: 126ms (PropertyUtils.getValue)
        Reflection setter cumulative time: 174ms (Custom implemetation using reflection to call bean setters)

        Sun JDK 5
        OGNL getter cumulative time: 14,540ms > (PropertyUtils.getValueOgnl) < (Notice how slow this runs in JDK5)
        OGNL setter cumulative time: 46,116ms > (PropertyUtils.setValueOgnl) < (Notice how slow this runs in JDK5)

        MVEL getter cumulative time: 103ms
        MVEL setter cumulative time: 24,649ms <- (Notice how slow this runs in JDK5)

        Reflection getter cumulative time: 94ms
        Reflection setter cumulative time: 102ms

        Not 100% sure why OGNL and MVEL setter runs slowly in JDK5. I remember from a previous test that the slowdown comes from using synchronization.

        Sun JDK 6 (The synchronization issue is gone)
        OGNL getter cumulative time: 1,985ms
        OGNL setter cumulative time: 6,768ms

        MVEL getter cumulative time: 48ms
        MVEL setter cumulative time: 1,485ms

        Reflection getter cumulative time: 48ms
        Reflection setter cumulative time: 64ms

        After all that it really does seem like reflection is the safest bet (taking both getters and setters into consideration) even tho our reflection implementation is rather limited.

        Perhaps we should ditch OGNL completely and make the binding pluggable by using simple inheritance?

        Also there is a JSR to standardize on a Unified expression language as part of JSP2.1: http://www.jcp.org/en/jsr/detail?id=245

        We can expect a few more EL implementations after that

        Show
        Bob Schellink added a comment - If you recall the test ran 20 threads for 1000 iterations. "Caching cumulative" means there is a global cache where compiled expressions are stored. Because the cache is global we needed to synchronize on the cache. "Thread caching cumulative" means there is a cache per thread and no synchronization is needed. Basically only "Thread caching cumulative" matters since we do not synchronize in Click. Below are results of the test with JDK 1.4, 1.5 and 1.6. I have also added tests for setting the value of beans. Windows XP, Dual core machine. Note the results below are after 2 warmup runs. Sun JDK 1.4 OGNL getter cumulative time: 1,970ms -> (PropertyUtils.getValueOgnl) OGNL setter cumulative time: 9,105ms -> (PropertyUtils.setValueOgnl) MVEL getter cumulative time: 153ms MVEL setter cumulative time: 2,390ms Reflection getter cumulative time: 126ms (PropertyUtils.getValue) Reflection setter cumulative time: 174ms (Custom implemetation using reflection to call bean setters) Sun JDK 5 OGNL getter cumulative time: 14,540ms > (PropertyUtils.getValueOgnl) < (Notice how slow this runs in JDK5) OGNL setter cumulative time: 46,116ms > (PropertyUtils.setValueOgnl) < (Notice how slow this runs in JDK5) MVEL getter cumulative time: 103ms MVEL setter cumulative time: 24,649ms <- (Notice how slow this runs in JDK5) Reflection getter cumulative time: 94ms Reflection setter cumulative time: 102ms Not 100% sure why OGNL and MVEL setter runs slowly in JDK5. I remember from a previous test that the slowdown comes from using synchronization. Sun JDK 6 (The synchronization issue is gone) OGNL getter cumulative time: 1,985ms OGNL setter cumulative time: 6,768ms MVEL getter cumulative time: 48ms MVEL setter cumulative time: 1,485ms Reflection getter cumulative time: 48ms Reflection setter cumulative time: 64ms After all that it really does seem like reflection is the safest bet (taking both getters and setters into consideration) even tho our reflection implementation is rather limited. Perhaps we should ditch OGNL completely and make the binding pluggable by using simple inheritance? Also there is a JSR to standardize on a Unified expression language as part of JSP2.1: http://www.jcp.org/en/jsr/detail?id=245 We can expect a few more EL implementations after that
        Hide
        mike brock added a comment -

        As the MVEL author, I would like to note, that a large part of why the MVEL JAR is as big as it is, is because we package the source code inside the JAR as well.

        If you remove the source code from the JAR, MVEL is around a 350KB JAR.

        MVEL is larger than OGNL because it has an inline JIT and three different operation runtimes (interpreted, reflection-optimized, bytecode-optimized)

        Show
        mike brock added a comment - As the MVEL author, I would like to note, that a large part of why the MVEL JAR is as big as it is, is because we package the source code inside the JAR as well. If you remove the source code from the JAR, MVEL is around a 350KB JAR. MVEL is larger than OGNL because it has an inline JIT and three different operation runtimes (interpreted, reflection-optimized, bytecode-optimized)
        Hide
        Malcolm Edgar added a comment -

        Bob, thanks for performance testing. Could you upload the test harness you used for this.

        Mike, thanks for the feedback on the library size.

        I think the MVEL library looks pretty promising for Click version 1.5. Its got comparable performance our reflection getter code, 3x the setter performance of our OGNL code, and also provides an expression language.

        I can't really see any downsides here. Some people may have some dependencies on OGNL, but since the change to reflection code I would be surprised.

        regards Malcolm Edgar

        Show
        Malcolm Edgar added a comment - Bob, thanks for performance testing. Could you upload the test harness you used for this. Mike, thanks for the feedback on the library size. I think the MVEL library looks pretty promising for Click version 1.5. Its got comparable performance our reflection getter code, 3x the setter performance of our OGNL code, and also provides an expression language. I can't really see any downsides here. Some people may have some dependencies on OGNL, but since the change to reflection code I would be surprised. regards Malcolm Edgar
        Hide
        Bob Schellink added a comment -

        Hi Malcolm,

        I have uploaded the benchmark. There is a README.txt indicating the dependent libraries and which class is the actual test

        If MVEL is used as the default expression language we will have to look at adding an expression variable to Controls. Currently a Controls name variable is used for both the name and expression of a Control. This is fine for simple bindings eg "name" or "address.street". However if we expose a full expression language, one can easily create an invalid HTML name, for example:

        Column c = new Column("price1 * price2 + price3 / price4");

        The columns name will now be "price1 * price2 + price3 / price4".

        We should rather look at adding an expression variable eg:

        public Column extends AbstractControl

        { private String expression; }

        To keep backward compatibility we can fallback to name if expression is null.

        Show
        Bob Schellink added a comment - Hi Malcolm, I have uploaded the benchmark. There is a README.txt indicating the dependent libraries and which class is the actual test If MVEL is used as the default expression language we will have to look at adding an expression variable to Controls. Currently a Controls name variable is used for both the name and expression of a Control. This is fine for simple bindings eg "name" or "address.street". However if we expose a full expression language, one can easily create an invalid HTML name, for example: Column c = new Column("price1 * price2 + price3 / price4"); The columns name will now be "price1 * price2 + price3 / price4". We should rather look at adding an expression variable eg: public Column extends AbstractControl { private String expression; } To keep backward compatibility we can fallback to name if expression is null.
        Hide
        Bob Schellink added a comment -

        Oops I should have uploaded a zip not rar.

        Anyway if anybody wants to have a look at the benchmark, both attachments contains the same content.

        Show
        Bob Schellink added a comment - Oops I should have uploaded a zip not rar. Anyway if anybody wants to have a look at the benchmark, both attachments contains the same content.
        Hide
        mike brock added a comment -

        I should note also, that due to your performance benchmarks on the setter performance, I have isolated the problem and will have a fix in the next dot release of MVEL 1.2.

        The problem has to do with the coercion engine and defensively passing the value to the engine to ensure a safe value write.

        The new performance spread of get versus set in trunk is about 1:4.

        Show
        mike brock added a comment - I should note also, that due to your performance benchmarks on the setter performance, I have isolated the problem and will have a fix in the next dot release of MVEL 1.2. The problem has to do with the coercion engine and defensively passing the value to the engine to ensure a safe value write. The new performance spread of get versus set in trunk is about 1:4.
        Hide
        Malcolm Edgar added a comment -

        Hi Mike, thanks for keeping us posted on the update. Can you tell me what the previous performance spread of get versus set was before this patch.

        Also is an extension point in MVEL to help in type conversion. One thing we have in web frameworks is everything coming in as a String and we need to convert values to other types, so it is handy to help out with this, i.e. convert "2008-01-21" into a date. With OGNL you can do this by providing a TypeConverter class.

        The other thing which OGNL does not address it handling of BigDecimal's additional scale and rounding properties. What I think is needed in coercion code examines the existing BigDecimal property to determine the scale and rounding, and apply these to any created BigDecimal values.

        regards Malcolm Edgar

        Show
        Malcolm Edgar added a comment - Hi Mike, thanks for keeping us posted on the update. Can you tell me what the previous performance spread of get versus set was before this patch. Also is an extension point in MVEL to help in type conversion. One thing we have in web frameworks is everything coming in as a String and we need to convert values to other types, so it is handy to help out with this, i.e. convert "2008-01-21" into a date. With OGNL you can do this by providing a TypeConverter class. The other thing which OGNL does not address it handling of BigDecimal's additional scale and rounding properties. What I think is needed in coercion code examines the existing BigDecimal property to determine the scale and rounding, and apply these to any created BigDecimal values. regards Malcolm Edgar
        Hide
        Bob Schellink added a comment -

        Thanks Mike, I will redo the benchmark for the next MVEL release.

        I just ran the tests for IBM JDK 1.4 and 1.5 with no issues.

        Also note that the slowdown I notice for Sun JDK5 is only visible on Windows not Linux. At least not 64bit Linux.

        Show
        Bob Schellink added a comment - Thanks Mike, I will redo the benchmark for the next MVEL release. I just ran the tests for IBM JDK 1.4 and 1.5 with no issues. Also note that the slowdown I notice for Sun JDK5 is only visible on Windows not Linux. At least not 64bit Linux.
        Hide
        mike brock added a comment -

        Malcom,

        Yes, you can provide your own type convertors in MVEL. Michael Neale of the JBoss Drools project implemented a string-date to date convertor in MVEL, no problem. There is not a default one in the 1.2 distribution (but there will be in 2.0). You can add your own converters by implementing a ConversionHandler and registering it in the DataConversion factory. Same basic deal as OGNL's TypeConverters.

        MVEL already uses a best-fit algorithm for scaling of numbers, all the way up to using BigDecimal. MVEL also supports implementing your own MathProcessor, and comes with two default implementations JDK14CompatibilityMath, and IEEEFloatingPointmath that implement the appropriate measures for handling this.

        Show
        mike brock added a comment - Malcom, Yes, you can provide your own type convertors in MVEL. Michael Neale of the JBoss Drools project implemented a string-date to date convertor in MVEL, no problem. There is not a default one in the 1.2 distribution (but there will be in 2.0). You can add your own converters by implementing a ConversionHandler and registering it in the DataConversion factory. Same basic deal as OGNL's TypeConverters. MVEL already uses a best-fit algorithm for scaling of numbers, all the way up to using BigDecimal. MVEL also supports implementing your own MathProcessor, and comes with two default implementations JDK14CompatibilityMath, and IEEEFloatingPointmath that implement the appropriate measures for handling this.
        Hide
        Bob Schellink added a comment -

        will resolve this in 1.5-M2

        Show
        Bob Schellink added a comment - will resolve this in 1.5-M2
        Hide
        Bob Schellink added a comment -

        Removing this issues from our current roadmap. Can revisit this issue in a future release

        Show
        Bob Schellink added a comment - Removing this issues from our current roadmap. Can revisit this issue in a future release
        Hide
        Adrian A. added a comment -

        -1 for this. As it works now is very good - no need for mvel
        (I would have voted, but JIRA allows only positive votes ).

        Show
        Adrian A. added a comment - -1 for this. As it works now is very good - no need for mvel (I would have voted, but JIRA allows only positive votes ).
        Hide
        Malcolm Edgar added a comment -

        I think backward compatibility issues preclude us from changing to MVEL.

        I recommend closing this issue.

        regards Malcolm

        Show
        Malcolm Edgar added a comment - I think backward compatibility issues preclude us from changing to MVEL. I recommend closing this issue. regards Malcolm
        Hide
        Malcolm Edgar added a comment -

        Currently failing unit test:

        PageTest.testRequestParameterBinding() when trying to convert a "true" string value to a Page boolean property.

        May need to register a type converter:

        http://mvel.codehaus.org/Type+Converters

        DataConversion.addConversionHandler(Boolean.class, new CustomConverter());

        Show
        Malcolm Edgar added a comment - Currently failing unit test: PageTest.testRequestParameterBinding() when trying to convert a "true" string value to a Page boolean property. May need to register a type converter: http://mvel.codehaus.org/Type+Converters DataConversion.addConversionHandler(Boolean.class, new CustomConverter());
        Hide
        Andrew Fink added a comment -

        +100
        Also MVEL template engine can be used for internal/components templates, so velocity (and many it's dependencies) can be removed from distribution.

        Show
        Andrew Fink added a comment - +100 Also MVEL template engine can be used for internal/components templates, so velocity (and many it's dependencies) can be removed from distribution.
        Hide
        Gilberto C Andrade added a comment -

        Digging a little in the issue, I've found the class[1] that make MVEL complain about permission as you can see in the log:
        <quote>
        [junit] at org.apache.tools.ant.taskdefs.optional.junit.JUnitTestRunner.main(JUnitTestRunner.java:906)
        [junit] Caused by: [Error: could not access property (bool) in: java.lang.String]
        [junit] [Near :

        {... bool ....}

        ]
        [junit] ^
        [junit] [Line: 1, Column: 1]
        [junit] at org.mvel2.optimizers.impl.refl.ReflectiveAccessorOptimizer.optimizeSetAccessor(ReflectiveAccessorOptimizer.java:312)
        [junit] at org.mvel2.optimizers.dynamic.DynamicOptimizer.optimizeSetAccessor(DynamicOptimizer.java:78)
        [junit] at org.mvel2.compiler.CompiledAccExpression.setValue(CompiledAccExpression.java:54)
        [junit] at org.mvel2.ast.DeepAssignmentNode.getReducedValueAccelerated(DeepAssignmentNode.java:91)
        [junit] at org.mvel2.compiler.ExecutableAccessor.getValue(ExecutableAccessor.java:42)
        [junit] at org.mvel2.MVEL.executeExpression(MVEL.java:969)

        </quote>

        If you change the access modifier of all fields(in the RequestBindingPage.java class) from private to public the test will pass.

        Thanks and good work,

        Gilberto

        [1] http://svn.apache.org/viewvc/click/trunk/click/framework/test/org/apache/click/pages/RequestBindingPage.java?revision=991908&view=markup

        Show
        Gilberto C Andrade added a comment - Digging a little in the issue, I've found the class [1] that make MVEL complain about permission as you can see in the log: <quote> [junit] at org.apache.tools.ant.taskdefs.optional.junit.JUnitTestRunner.main(JUnitTestRunner.java:906) [junit] Caused by: [Error: could not access property (bool) in: java.lang.String] [junit] [Near : {... bool ....} ] [junit] ^ [junit] [Line: 1, Column: 1] [junit] at org.mvel2.optimizers.impl.refl.ReflectiveAccessorOptimizer.optimizeSetAccessor(ReflectiveAccessorOptimizer.java:312) [junit] at org.mvel2.optimizers.dynamic.DynamicOptimizer.optimizeSetAccessor(DynamicOptimizer.java:78) [junit] at org.mvel2.compiler.CompiledAccExpression.setValue(CompiledAccExpression.java:54) [junit] at org.mvel2.ast.DeepAssignmentNode.getReducedValueAccelerated(DeepAssignmentNode.java:91) [junit] at org.mvel2.compiler.ExecutableAccessor.getValue(ExecutableAccessor.java:42) [junit] at org.mvel2.MVEL.executeExpression(MVEL.java:969) </quote> If you change the access modifier of all fields(in the RequestBindingPage.java class) from private to public the test will pass. Thanks and good work, Gilberto [1] http://svn.apache.org/viewvc/click/trunk/click/framework/test/org/apache/click/pages/RequestBindingPage.java?revision=991908&view=markup
        Hide
        Gilberto C Andrade added a comment - - edited

        Have you seen that if you remove @Bindable annotation the test runs without MVEL exception as well. I think the @Bindable annotation is the problem.

        To confirm that @Bindable can be the problem I'm quoting Mike Brock's idea of encapsulation: "But that's because the method is private, and MVEL only looks for properly accessible methods. MVEL has always respected class security and encapsulation rules."[1]

        Regards,

        Gilberto
        [1] http://jira.codehaus.org/browse/MVEL-186

        Show
        Gilberto C Andrade added a comment - - edited Have you seen that if you remove @Bindable annotation the test runs without MVEL exception as well. I think the @Bindable annotation is the problem. To confirm that @Bindable can be the problem I'm quoting Mike Brock's idea of encapsulation: "But that's because the method is private, and MVEL only looks for properly accessible methods. MVEL has always respected class security and encapsulation rules." [1] Regards, Gilberto [1] http://jira.codehaus.org/browse/MVEL-186
        Hide
        Andrew Fink added a comment -

        Malcolm Edgar wrote:
        I think there are a couple of options here:
        1. Rollback the changes to use OGNL,
        2. Replace OGNL with MVEL, and stop supporting access to private variables (could break a fair bit of code)
        3. Have a pluggable PropertyService with OGNL and MVEL implementations (what should be the default?)

        Of these options I would prefer #3.
        ===

        Pluggable PropertyService is very good idea! One can implement then Spring's SpEL implementation too.

        Show
        Andrew Fink added a comment - Malcolm Edgar wrote: I think there are a couple of options here: 1. Rollback the changes to use OGNL, 2. Replace OGNL with MVEL, and stop supporting access to private variables (could break a fair bit of code) 3. Have a pluggable PropertyService with OGNL and MVEL implementations (what should be the default?) Of these options I would prefer #3. === Pluggable PropertyService is very good idea! One can implement then Spring's SpEL implementation too.
        Hide
        Gilberto C Andrade added a comment -

        Ok, I've found the problem. To make the test pass we need to add the sets/gets in the RequestBindingPage class. The patch make use of the mvel2.version=2.0.19.

        Hth,

        Gilberto

        Show
        Gilberto C Andrade added a comment - Ok, I've found the problem. To make the test pass we need to add the sets/gets in the RequestBindingPage class. The patch make use of the mvel2.version=2.0.19. Hth, Gilberto
        Hide
        Bob Schellink added a comment -

        Yep that would make the test pass, however the test is about ensuring that the binding occurs on private variables. OGNL supports this feature, MVEL does not which is why we cannot simply replace OGNL with MVEL as it could break existing applications.

        Show
        Bob Schellink added a comment - Yep that would make the test pass, however the test is about ensuring that the binding occurs on private variables. OGNL supports this feature, MVEL does not which is why we cannot simply replace OGNL with MVEL as it could break existing applications.
        Hide
        Malcolm Edgar added a comment -

        I think the solution to this is to introduce a pluggable PropertyService. I will work on this unless anyone thinks otherwise, in terms of the default service I think for the next release we should probably default to OGNL and in a following release we can change this to MVEL once we get some more experience.

        Show
        Malcolm Edgar added a comment - I think the solution to this is to introduce a pluggable PropertyService. I will work on this unless anyone thinks otherwise, in terms of the default service I think for the next release we should probably default to OGNL and in a following release we can change this to MVEL once we get some more experience.
        Hide
        jack hexy added a comment -

        hi,make an effort,I wish you succeed and enjoy in it,with best regards!

        Show
        jack hexy added a comment - hi,make an effort,I wish you succeed and enjoy in it,with best regards!
        Hide
        Gilberto C Andrade added a comment -

        I'm getting this RunTimeException: No Context available on ThreadLocal Context Stack while building the framework. The fix is to add the following statement in some test files:
        MockContext context = MockContext.initContext();

        Show
        Gilberto C Andrade added a comment - I'm getting this RunTimeException: No Context available on ThreadLocal Context Stack while building the framework. The fix is to add the following statement in some test files: MockContext context = MockContext.initContext();
        Hide
        Gilberto C Andrade added a comment - - edited

        I thought MVEL would be more performatic!
        But:

        gilberto.andrade@A37710:~/bin/click-trunk$ uname -a
        Linux A37710.ADMINISTRACAO 3.7.10-1.1-desktop #1 SMP PREEMPT Thu Feb 28 15:06:29 UTC 2013 (82d3f21) x86_64 x86_64 x86_64 GNU/Linux
        gilberto.andrade@A37710:~/bin/click-trunk$ javac -version
        javac 1.7.0_17
        gilberto.andrade@A37710:~/bin/click-trunk$ javac -version
        
        
        gilberto.andrade@A37710:~/bin/click-trunk$ cat framework/classes/TEST-org.apache.click.service.PropertyServicePerformanceTest.txt
        Testsuite: org.apache.click.service.PropertyServicePerformanceTest
        Tests run: 2, Failures: 0, Errors: 0, Time elapsed: 20,103 sec
        ------------- Standard Error -----------------
        OGNLPropertyService cumulative  read test in 59599 ms 
        OGNLPropertyService cumulative write test in 27741 ms 
        MVELPropertyService cumulative  read test in 97231 ms 
        MVELPropertyService cumulative write test in 27145 ms 
        ------------- ---------------- ---------------
        
        Testcase: test_OGNLService took 10,017 sec
        Testcase: test_MVELService took 10,079 sec
        
        

        Bob, would you mind to make your test again?

        regards

        Show
        Gilberto C Andrade added a comment - - edited I thought MVEL would be more performatic! But: gilberto.andrade@A37710:~/bin/click-trunk$ uname -a Linux A37710.ADMINISTRACAO 3.7.10-1.1-desktop #1 SMP PREEMPT Thu Feb 28 15:06:29 UTC 2013 (82d3f21) x86_64 x86_64 x86_64 GNU/Linux gilberto.andrade@A37710:~/bin/click-trunk$ javac -version javac 1.7.0_17 gilberto.andrade@A37710:~/bin/click-trunk$ javac -version gilberto.andrade@A37710:~/bin/click-trunk$ cat framework/classes/TEST-org.apache.click.service.PropertyServicePerformanceTest.txt Testsuite: org.apache.click.service.PropertyServicePerformanceTest Tests run: 2, Failures: 0, Errors: 0, Time elapsed: 20,103 sec ------------- Standard Error ----------------- OGNLPropertyService cumulative read test in 59599 ms OGNLPropertyService cumulative write test in 27741 ms MVELPropertyService cumulative read test in 97231 ms MVELPropertyService cumulative write test in 27145 ms ------------- ---------------- --------------- Testcase: test_OGNLService took 10,017 sec Testcase: test_MVELService took 10,079 sec Bob, would you mind to make your test again? regards
        Hide
        Gilberto C Andrade added a comment -

        I've found the problem, I think:

        PropertyServicePerformanceTest.java
        	public void test_MVELService() throws Exception {
        		PropertyService ps = new MVELPropertyService();
        
        		for (int i = 0; i < 100; i++) {
        			Thread testThread = new Thread(new TestRunner(ps, i == 49));
        			testThread.start();
        		}
        		
        		Thread.sleep(10000);
        	}
        	
        	public void test_OGNLService() throws Exception {
        		readDuration.set(0);
        		writeDuration.set(0);
        		
        		PropertyService ps = new OGNLPropertyService();
        
        		for (int i = 0; i < 50; i++) {
        			Thread testThread = new Thread(new TestRunner(ps, i == 49));
        			testThread.start();
        		}
        		
        		Thread.sleep(10000);
        	}
        
        

        The for loop in the MVEL test has 100 as limit while in the OGNL has only 50. And this complex code to test the performance has some problem: if you run it twice it will invert the winner.

        Show
        Gilberto C Andrade added a comment - I've found the problem, I think: PropertyServicePerformanceTest.java public void test_MVELService() throws Exception { PropertyService ps = new MVELPropertyService(); for ( int i = 0; i < 100; i++) { Thread testThread = new Thread ( new TestRunner(ps, i == 49)); testThread.start(); } Thread .sleep(10000); } public void test_OGNLService() throws Exception { readDuration.set(0); writeDuration.set(0); PropertyService ps = new OGNLPropertyService(); for ( int i = 0; i < 50; i++) { Thread testThread = new Thread ( new TestRunner(ps, i == 49)); testThread.start(); } Thread .sleep(10000); } The for loop in the MVEL test has 100 as limit while in the OGNL has only 50. And this complex code to test the performance has some problem: if you run it twice it will invert the winner.

          People

          • Assignee:
            Malcolm Edgar
            Reporter:
            Bob Schellink
          • Votes:
            2 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

            • Created:
              Updated:

              Development