What does this require plugin authors to do? If this requires changes in plugins then it’s a definite non-starter imho
Please write tests that prove the performance difference is real, talking about it is great, but we need to know ‘how much’ faster this is to make a decision. I also don’t think making something Object Oriented is a good enough reason to make a change. It could be argued that it does not make it any more oop. > On Apr 30, 2018, at 2:47 AM, Wojciech Trocki <wtro...@gmail.com> wrote: > > Really interesting topic. > I'm particularly interested how newest versions of Java (and Android > platform) handle reflection. > Happy to update my PR upon wider agreement, but I'm still concerned about > performance. > This change can help plugin developers but also could slow down end user > apps. > >> method invokation can be optimized with new Java 7's MethodHandle > > I haven't used that in my investigation before, so going to check that. > >> method lookup can be do only single time, on the first call of method > execute for a plugin. > > I have tried to do it when plugin is initialized which should not affect > end users. > Doing that on first call can introduce some hard to detect bugs on end user > apps. > For example when 2 calls are made at the same time, before waiting for > callback etc. > > > On Mon, Apr 30, 2018 at 10:27 AM, chemeri...@gmail.com <chemeri...@gmail.com >> wrote: > >> I still prefer using annotation: >> >> 1) all logic is still in one class (unlike OO approch where new helper >> classes introduced) >> 2) it's visually clearer which methods are mapped to an appropriate >> cordova call. >> >> As for performance: >> 1) method lookup can be do only single time, on the first call of method >> execute for a plugin. Mapping of action to method can be stored internally, >> so later calls won't touch reflection >> 2) method invokation can be optimized with new Java 7's MethodHandle ( >> https://docs.oracle.com/javase/7/docs/api/java/lang/ >> invoke/MethodHandle.html) class. >> >>> On 2018/04/29 15:37:51, Joe Bowser <bows...@gmail.com> wrote: >>> Cordova should be reducing the use of Reflection, not increasing it. I >>> don't think this is a good idea. >>> >>>> On Sun, Apr 29, 2018, 8:28 AM Jesse, <purplecabb...@gmail.com> wrote: >>>> >>>> I would like to see proof of value. >>>> I believe the lookup of an action is insignificant compared to the >> message >>>> conversion between js and native. >>>> Please write some tests to justify your position. >>>> >>>> >>>>> On Apr 29, 2018, at 7:59 AM, julio cesar sanchez < >> jcesarmob...@gmail.com> >>>> wrote: >>>>> >>>>> I think it's a good idea. >>>>> >>>>> FYI, there is a plugin that already allows this, you just have to >> add it >>>> as >>>>> a dependency, but it's not backward compatible >>>>> https://github.com/edewit/aerogear-reflect-cordova >>>>> >>>>> 2018-04-29 16:44 GMT+02:00 Wojciech Trocki <wtro...@redhat.com>: >>>>> >>>>>> Hi Maksim >>>>>> >>>>>> `if (METHOD_1.equals(action))` is very similar way to how redux >> works. >>>>>> >>>>>> I have playing with your proposition to see how this could be >>>> implemented. >>>>>> What I found is that processing annotations on runtime can >> contribute to >>>>>> slower application startup due to fact that annotation needs to be >>>>>> processed at runtime. >>>>>> Deviceready event is delivered later, which is not desired. This >> could >>>> be >>>>>> also processed on build time (something like dagger), but I did not >>>>>> investigated that. >>>>>> >>>>>> I figured out simple way to extend this without any sacrifice on >>>>>> performance. >>>>>> This will simplify end user api and also still provide full >> backwards >>>>>> compatibility. >>>>>> We could make improvement for developers without affecting end user >> app >>>>>> performance. >>>>>> >>>>>> Linking gist with the idea: >>>>>> https://gist.github.com/wtrocki/43bfdda18c086a3283bb8ba3bf2d052e >>>>>> >>>>>> Happy to contribute that back to the Cordova. >>>>>> >>>>>> *Note: *I'm not maintainer of Cordova Android platform so my >> response do >>>>>> not mean that there will be approval for this change. >>>>>> >>>>>> >>>>>> On Sun, Apr 29, 2018 at 9:58 AM, chemeri...@gmail.com < >>>>>> chemeri...@gmail.com> >>>>>> wrote: >>>>>> >>>>>>> Hi guys. cordova-ios has a nice method binding for plugins. >>>> Unfortunately >>>>>>> cordova-android requires at present boilerplate implementation of >>>> method >>>>>>> execute: >>>>>>> >>>>>>> public class MyPlugin extends CordovaPlugin { >>>>>>> ... >>>>>>> @Override >>>>>>> public boolean execute(String action, JSONArray args, >>>> CallbackContext >>>>>>> callbackContext) throws JSONException { >>>>>>> if (METHOD_1.equals(action)) { >>>>>>> method1(args, callbackContext); >>>>>>> } else if (METHOD_2.equals(action)) { >>>>>>> method2(args, callbackContext); >>>>>>> ... >>>>>>> } else { >>>>>>> return false; >>>>>>> } >>>>>>> return true; >>>>>>> } >>>>>>> ... >>>>>>> } >>>>>>> >>>>>>> I suggest to implement support for @CordovaMethod that will allow >> for >>>>>>> plugin authors to reduce boilerplate code, so the implementation >> above >>>>>> will >>>>>>> look like: >>>>>>> >>>>>>> public class MyPlugin extends CordovaPlugin { >>>>>>> ... >>>>>>> @CordovaMethod >>>>>>> private void method1(JSONArray args, CallbackContext >>>> callbackContext) >>>>>>> throws JSONException { >>>>>>> ... >>>>>>> } >>>>>>> >>>>>>> @CordovaMethod >>>>>>> private void method2(JSONArray args, CallbackContext >>>> callbackContext) >>>>>>> throws JSONException { >>>>>>> ... >>>>>>> } >>>>>>> ... >>>>>>> } >>>>>>> >>>>>>> Implementation of method execute in CordovaPlugin.java will check >> for >>>>>>> methods marked with @CordovaMethod and if action argument matches a >>>>>> method >>>>>>> with @CordovaMethod, the method will be invoked. Developer can also >>>>>> specify >>>>>>> action manually via the name parameter: >>>>>>> >>>>>>> public class MyPlugin extends CordovaPlugin { >>>>>>> ... >>>>>>> @CordovaMethod(name = "method1") >>>>>>> private void myMethod(JSONArray args, CallbackContext >>>>>> callbackContext) >>>>>>> throws JSONException { >>>>>>> ... >>>>>>> } >>>>>>> ... >>>>>>> } >>>>>>> >>>>>>> Important to note that backward compatibility is preserved - old >>>> plugins >>>>>>> didn't have @CordovaMethod, but new ones can use it to simplify >> code. >>>>>>> >>>>>>> What do you think? >>>>>>> >>>>>>> >>>>>>> ------------------------------------------------------------ >> --------- >>>>>>> To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org >>>>>>> For additional commands, e-mail: dev-h...@cordova.apache.org >>>>>>> >>>>>>> >>>>>> >>>>>> >>>>>> -- >>>>>> >>>>>> WOJCIECH TROCKI >>>>>> >>>>>> Red Hat Mobile <https://www.redhat.com/> >>>>>> >>>>>> IM: wtrocki >>>>>> <https://red.ht/sig> >>>>>> >>>> >>>> --------------------------------------------------------------------- >>>> To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org >>>> For additional commands, e-mail: dev-h...@cordova.apache.org >>>> >>>> >>> >> >> --------------------------------------------------------------------- >> To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org >> For additional commands, e-mail: dev-h...@cordova.apache.org >> >> --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org For additional commands, e-mail: dev-h...@cordova.apache.org
