Changing the name of all the JNI functions

[Daniel Bratell]

In the ongoing jumbo project, we noticed that there are around 100 methods
named Init() method coming from the JNI system (Java <-> C bridge). Since
this was not good (currently a dozen files are excluded from
content/browser jumbo for this reason), I looked at renaming then
ClassName__Init() and it's possible.

Quite naturally it doesn't seem logical to rename just the Init() methods
when there are other JNI methods so it was recommended they should all be
renamed. It's a big patch! (See
https://chromium-review.googlesource.com/c/chromium/src/+/765973 )

So I wonder:

A) Should all methods be renamed or just Init()?
B) What should the naming scheme be?
C) Is there an option I'm missing here?

My opinions are:

A) No strong feelings though this is a sidetrack so unless someone
volunteers, nothing that requires massive work.

B) Again, no strong feelings. I used ClassName__FunctionName, torne
suggested something with a JNI_ prefix, maybe JNI_ClassName_FunctionName.

C) I wouldn't ask if I had an answer, would I?

/Daniel

--
/* Opera Software, Linköping, Sweden: CET (UTC+1) */

[Dmitry Skiba]

Hi,

ClassName__methodName looks a bit ugly to me - why not put methods to ClassName namespace instead? Or <ClassName>JNI to stress its JNI nature.

Dmitry

--
--
Chromium Developers mailing list: chromi...@chromium.org
View archives, change email options, or unsubscribe:   http://groups.google.com/a/chromium.org/group/chromium-dev
---You received this message because you are subscribed to the Google Groups "Chromium-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to chromium-dev+unsubscribe@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/chromium-dev/op.y9reljj5rbppqq%40cicero2.linkoping.osa.

[Dmitry Skiba]

Another idea: we can hide the ugliness behind a macro: let's say generated JNI headers will include a macro like this:

#define JNI_METHOD(name) ClassName__##name

Then in the actual implementation file we would just write

static void JNI_METHOD(foo)(...) {}

Side benefit is that JNI methods are clearly marked.

[Daniel Bratell]

Namespaces would be an alternative but it falls a bit on the implementation phase. It's not something that can be (easily) automatically implemented and I don't think we'll find anyone motivated enough to do it manually.

The macro would make jumbo unhappy because you would suddenly have a dozen different "JNI_METHOD" macros around. Actually there are already a few files that implement 2 or 3 JNI interfaces. They would also not be happy.

But I guess from your answers that you think that renaming the methods, all of them, might be a good idea, but that the current naming (which is kind of a placeholder, but also intentionally reeks of generated code) is not as good as something that includes the string "JNI".

/Daniel

To unsubscribe from this group and stop receiving emails from it, send an email to chromium-dev...@chromium.org.

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/chromium-dev/op.y9reljj5rbppqq%40cicero2.linkoping.osa.

[Andrew Grieve]

I'm certainly supportive of prefixing these. As for tacking "JNI" onto the name, I'm happy either way. "JNI" lets you know that it's a generate JNI call, but ClassName_Foo() is also pretty clear I think.

To unsubscribe from this group and stop receiving emails from it, send an email to chromium-dev+unsubscribe@chromium.org.

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/chromium-dev/op.y9reljj5rbppqq%40cicero2.linkoping.osa.

--

/* Opera Software, Linköping, Sweden: CET (UTC+1) */

--

--
Chromium Developers mailing list: chromi...@chromium.org
View archives, change email options, or unsubscribe:
http://groups.google.com/a/chromium.org/group/chromium-dev

---
You received this message because you are subscribed to the Google Groups "Chromium-dev" group.

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/chromium-dev/op.y9slg80nrbppqq%40cicero2.linkoping.osa.

[Dmitry Skiba]

On Thu, Nov 16, 2017 at 2:44 AM, Daniel Bratell <bra...@opera.com> wrote:

Namespaces would be an alternative but it falls a bit on the implementation phase. It's not something that can be (easily) automatically implemented and I don't think we'll find anyone motivated enough to do it manually.

So changes in your CL were generated? How did you generate them? Can that script be adapted to add namespaces instead?

 

The macro would make jumbo unhappy because you would suddenly have a dozen different "JNI_METHOD" macros around. Actually there are already a few files that implement 2 or 3 JNI interfaces. They would also not be happy.

Macro should be fine, it'll get overwritten by next ClassName_jni.h file. But I agree that files that implement multiple JNI interfaces will be screwed (since in that case we have multiple _jni.h includes in succession without implementation between them).

 

But I guess from your answers that you think that renaming the methods, all of them, might be a good idea, but that the current naming (which is kind of a placeholder, but also intentionally reeks of generated code) is not as good as something that includes the string "JNI".

I like your latest proposal from the CL (JNI_ClassName_foo), I think it looks better than ClassName__foo.

 

/Daniel

To unsubscribe from this group and stop receiving emails from it, send an email to chromium-dev+unsubscribe@chromium.org.

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/chromium-dev/op.y9reljj5rbppqq%40cicero2.linkoping.osa.

[Andrew Grieve]

On Thu, Nov 16, 2017 at 11:54 AM, 'Dmitry Skiba' via Chromium-dev <chromi...@chromium.org> wrote:

On Thu, Nov 16, 2017 at 2:44 AM, Daniel Bratell <bra...@opera.com> wrote:

Namespaces would be an alternative but it falls a bit on the implementation phase. It's not something that can be (easily) automatically implemented and I don't think we'll find anyone motivated enough to do it manually.

So changes in your CL were generated? How did you generate them? Can that script be adapted to add namespaces instead?

I'm curious why you want a namespace? What namespace would you use?

 

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/chromium-dev/CABWS%2BBzPOrxnx36wX2joBgrwNnXB%3D2K1CVBV1P3DVOQP%2BzPoJA%40mail.gmail.com.

[Dmitry Skiba]

On Thu, Nov 16, 2017 at 9:20 AM, Andrew Grieve <agr...@chromium.org> wrote:

On Thu, Nov 16, 2017 at 11:54 AM, 'Dmitry Skiba' via Chromium-dev <chromi...@chromium.org> wrote:

On Thu, Nov 16, 2017 at 2:44 AM, Daniel Bratell <bra...@opera.com> wrote:

Namespaces would be an alternative but it falls a bit on the implementation phase. It's not something that can be (easily) automatically implemented and I don't think we'll find anyone motivated enough to do it manually.

So changes in your CL were generated? How did you generate them? Can that script be adapted to add namespaces instead?

I'm curious why you want a namespace? What namespace would you use?

Honestly I just don't boilerplate, I would like to specify class name once and just use it for all JNI functions. I.e. 

namespace ClassNameJNI {

void foo() {}

void bar() {}

}

This will also make diff smaller, as we'll have to add 2 lines per file. Java class name changes are also easier (just change the namespace name).

However, we already have ClassName::foo() all over C++ codebase, which kinda invalidates my point, since it's not much different from ClassName__foo().

I don't like ClassName__foo() because of the double underscore and unclear relation to JNI. Proposed JNI_ClassName_foo() fixes all that.

[Daniel Bratell]

On Thu, 16 Nov 2017 17:54:01 +0100, Dmitry Skiba <dsk...@google.com> wrote:

On Thu, Nov 16, 2017 at 2:44 AM, Daniel Bratell <bra...@opera.com> wrote:

Namespaces would be an alternative but it falls a bit on the implementation phase. It's not something that can be (easily) automatically implemented and I don't think we'll find anyone motivated enough to do it manually.

So changes in your CL were generated? How did you generate them? Can that script be adapted to add namespaces instead?

A script with regexps and simple string operations got it 90% right and then it was manual fixing and cleanup on top of that. You can peephole identify likely JNI methods by a combination of includes, return types and argument types and change the function name on the fly.

I was also helped by not knowing from the start how much there was to change.  :)