Re: Add support for annotation @CordovaMethod

2018-05-12 Thread chemerisuk
Just submitted PR with example implementation: https://github.com/apache/cordova-android/pull/440 On 2018/05/01 16:42:11, chemeri...@gmail.com wrote: > For anybody who is interested in what options java has to invoke methods I > recommend to read an interested link >

Re: Add support for annotation @CordovaMethod

2018-05-07 Thread chemerisuk
Slightly updated demo to use method.setAccessible(true) and made MethodHandles.Lookup to be static. The latest version is https://gist.github.com/chemerisuk/287e6cd6e1aa61a5680027967008b702. Numbers haven't changed a lot: OldCordovaPlugin: 0.03s with v=600 MethodCordovaPlugin: 0.14s with

Re: Add support for annotation @CordovaMethod

2018-05-05 Thread chemerisuk
I've created performance test for different @CordovaMethod implementations that allows to measure timings - https://gist.github.com/chemerisuk/287e6cd6e1aa61a5680027967008b702. On my machine results are like below: OldCordovaPlugin: 0.03s with v=600 MethodCordovaPlugin: 0.14s with

Re: Add support for annotation @CordovaMethod

2018-05-01 Thread chemerisuk
For anybody who is interested in what options java has to invoke methods I recommend to read an interested link https://stackoverflow.com/questions/19557829/faster-alternatives-to-javas-reflection. Accepted answer has useful information and demo class where different technics tested. On my

Re: Add support for annotation @CordovaMethod

2018-05-01 Thread Frederico Galvão
Although a relative performance metric is important and a good starting point, it's by far the most overused checkpoint to stop something from being considered in programming, when in reality absolute numbers and context are the truth. It doesn't matter if, from a arbitrary 200ms from activity

Re: Add support for annotation @CordovaMethod

2018-04-30 Thread Simon MacDonald
> > On Mon, Apr 30, 2018 at 12:37 PM Wojciech Trocki > wrote: > > Maybe I'm not understanding the goal here and it would be clearer with an > example, but it looks like this would split each action out into its own > class? I'm not sure what advantage there is to that, since

Re: Add support for annotation @CordovaMethod

2018-04-30 Thread Darryl Pogue
On Mon, Apr 30, 2018 at 12:37 PM Wojciech Trocki wrote: > > It's nice but it's just syntactic sugar so if it is slower than the > current > method I'd say no. > Linked PR[1] has no performance impact for end users. > It's not changing any current Cordova API etc. so it's

Re: Add support for annotation @CordovaMethod

2018-04-30 Thread Wojciech Trocki
> It's nice but it's just syntactic sugar so if it is slower than the current method I'd say no. Linked PR[1] has no performance impact for end users. It's not changing any current Cordova API etc. so it's pretty safe. As for original idea: On my testing devices most performant annotation reader

Re: Add support for annotation @CordovaMethod

2018-04-30 Thread Simon MacDonald
It's nice but it's just syntactic sugar so if it is slower than the current method I'd say no. If it impacts the speed of the current method I'd say no as well. So I'd like to see some performance tests of the various methods before going ahead with this. Simon On Mon, Apr 30, 2018, 15:13 Joe

Re: Add support for annotation @CordovaMethod

2018-04-30 Thread Joe Bowser
I'm fine with this. The problem we have now with our extensive use of reflection on Android is both with performance, as well as with being able to use obfuscation tools like ProGuard, which many auditors want to use to determine whether their code is "actually secure". I don't think obfuscation

Re: Add support for annotation @CordovaMethod

2018-04-30 Thread Wojciech Trocki
I have created PR in Android repository to show idea. https://github.com/apache/cordova-android/pull/439 This change do not use reflection/annotations mentioned in original proposition. IMHO with this change plugin interface will be much simpler and more object oriented. On Sun, Apr 29, 2018 at

Re: Add support for annotation @CordovaMethod

2018-04-30 Thread Jesse
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

Re: Add support for annotation @CordovaMethod

2018-04-30 Thread Wojciech Trocki
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. >

Re: Add support for annotation @CordovaMethod

2018-04-30 Thread chemerisuk
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

Re: Add support for annotation @CordovaMethod

2018-04-29 Thread Wojciech Trocki
I have created PR in Android repository to show idea. https://github.com/apache/cordova-android/pull/439 Aproach is not using reflection/annotations mentioned in original proposition. IMHO with this change plugin interface will be much simpler and more object oriented. On Sun, Apr 29, 2018 at

Re: Add support for annotation @CordovaMethod

2018-04-29 Thread Rabindra Nayak
How about iOS annotation @CordovaMethod.Because the way Android and iOS and other OS plugin implementation should be same.We need think that aspect. On Sun, Apr 29, 2018, 21:08 Joe Bowser wrote: > Cordova should be reducing the use of Reflection, not increasing it. I >

Re: Add support for annotation @CordovaMethod

2018-04-29 Thread Joe Bowser
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, wrote: > I would like to see proof of value. > I believe the lookup of an action is insignificant compared to the message >

Re: Add support for annotation @CordovaMethod

2018-04-29 Thread Jesse
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 > wrote: > > I

Re: Add support for annotation @CordovaMethod

2018-04-29 Thread julio cesar sanchez
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 : > Hi Maksim > > `if

Re: Add support for annotation @CordovaMethod

2018-04-29 Thread Wojciech Trocki
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

Add support for annotation @CordovaMethod

2018-04-29 Thread chemerisuk
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,