Closed Bug 279649 Opened 20 years ago Closed 20 years ago

Dynamically create Java proxies

Categories

(Core Graveyard :: Java to XPCOM Bridge, defect)

PowerPC
All
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: jhpedemonte, Assigned: jhpedemonte)

Details

Attachments

(2 files, 2 obsolete files)

Currently, both the Java interfaces and proxy stubs are created at build time
from the IDL files.  It would be better to have the proxies dynamically created
during runtime, using either java.lang.reflect.Proxy or Rhino's ClassFileWriter
(http://lxr.mozilla.org/mozilla/source/js/rhino/src/org/mozilla/classfile/ClassFileWriter.java).
 The Proxy class will be easier to use, but ClassFileWriter may have better
performance.
Yes, that could be used for what I am doing.  But I already have it more or less
working with Java's Proxy class.  Since I only need to create proxies from
interfaces, I don't need any of the extra features that cglib provides.  In
fact, I think the only reason to consider cglib or ClassFileWriter would be
performance issues.
Summary: Dyamically create Java proxies → Dynamically create Java proxies
This patch uses java.lang.reflect.Proxy to dynamically create the Java proxy
for a given XPCOM object.  The proxy stores a ref to the XPCOM object.	Also
removed support from the custom xpidl to create the 'javastubs'.  This cut down
the jar file size by about 60%.

Michal, if you've got the time, can you look over this, or even test it?  I
wanted to get this done before getting to bug 275359.
Attachment #172365 - Flags: review?(darin)
> Michal, if you've got the time, can you look over this, or even test it?  I 
> wanted to get this done before getting to bug 275359. 
this small fix was needed for me to compile it (tried on Linux) 
 
Now, I'll recreate the many-to-one patch from bug 275359 and 
try the proxies with my app. 
 
Attached patch compile fixSplinter Review
This seems like the right approach to me.  Shaver: I seem to recall that you had
some thoughts about this or that perhaps you remembered some gotchas concerning
this.  If you have any input, it'd be greatly appreciated.  Thanks!
(In reply to comment #4) 
> Now, I'll recreate the many-to-one patch from bug 275359 and  
> try the proxies with my app.  
So, patches from 275359 are ported now. 
I'll clean them and post again. 
 
I've tried with my app to iterate the DOM tree,  
access cookies, DOM serialization.  
Works fine for me. 
 
good work :-) 
 
Yeah, dynamic proxy generation is totally the right approach.  Let me look at
the patch in more detail when I get a chance -- man, I haven't read Java code in
ages!  I think the only piece that you need to be concerned about is the ability
to create "metadata-only" class stubs for Java developers to compile against,
because javac doesn't know how to ask you for the implementation of a given package.

Off in xpcom-dotnet land (soon to be extensions/mono, I think), I try to avoid
using the proxy interception capabilities, because performance there is pretty
dreadful.  I hook into what is the moral equivalent of the classloader system,
so that I can fully synthesize .NET objects that themselves call through my very
thin C++ xptcall wrapper, after doing the appropriate argument conversions. 
Because those conversions are known at class generation time, I can generate
some pretty JIT-friendly code, and I don't have to go through the reflection
APIs on any call but the first, for a given interface.  (.NET 1.0 doesn't let me
defer generation on a per-method basis, sadly.)

In the future, roc and I will probably try to reduce the in-memory size of the
generated code by creating a delegate for each distinct method signature, and
writing very thin methods that just pass the arguments, this, and a method index
to the target delegate.  (Or, perhaps, static method, which I can generate quite
quickly once the .NET 2.0 LWCG stuff is available.)

That was a bit of a braindump, sorry, but those are the design issues I'm
dealing with in a parallel space, so I thought they might be of interest.
Also: I don't know what guarantees Java makes about finalization and threads,
but it seems to me like you could call into finalizeProxy and find
gJavaXPCOMInitialized to be true while another thread was in the middle of
tearing down the various classes.  You might need an actual RW lock there to be
safe, or this might be a total false alarm because of Java's finalization semantics.
(In reply to comment #8)
> I think the only piece that you need to be concerned about is the ability
> to create "metadata-only" class stubs for Java developers to compile against,
> because javac doesn't know how to ask you for the implementation of a given
package.

Shaver, I'm not sure I understand what you mean here.  I already create Java
interfaces from the IDL files, which developers would link their Java app
against.  Is that not enough?
I thought we determined that using the IDL files made it very hard to give
proper type information for interface types, because of forward declarations. 
Using the same mechanism that's used for generating the proxies at runtime has
at least two advantages:

 - harder for the generated interfaces and proxies to get out of sync, and
 - you have fully resolved interface info available through the IIM

That's what we're doing over in extensions/mono, FWIW.
Oh, yeah, I intend to create a IIM-based utility to replace my current xpidl impl.
Attached patch patch v1.1 (obsolete) — Splinter Review
This patch adds a lock around |FreeGlobals()| and |finalizeProxy()|, since it
is possible that the garbage collector will be run on another thread.  Not sure
if I'm using the |PRLock| in the right way here.  The patch also makes |mIInfo|
from |JavaXPCOMInstance| be deleted by call to |NS_ProxyRelease|.

One thing that I have been noticing, though, is that every so often I get this
exception:

###!!! ASSERTION: Native event queues should only be used on the main thread:
'nsIThread::IsMainThread()', file
/home/pedemonte/builds/trunk/mozilla/xpcom/threads/nsEventQueue.cpp, line 230

Happens during the call to |NS_GetMainEventQ()|.  I thought the whole point of
that function was to get the main event queue from another thread.
Attachment #172365 - Attachment is obsolete: true
Darin, any idea about the assertion I mentioned in comment #13?  I think you
handled a bug dealing with this in the past.
Attached patch patch v1.2Splinter Review
Fixes assertion and cleans up |initEmbedding| and |initXPCOM|.	I was getting
the assertion because the main event queue was getting created on the garbage
collector thread.  But it should have been created much earlier.  So in this
patch, I create it during Java calls to |initXPCOM| and |initEmbedding|.
Attachment #173814 - Attachment is obsolete: true
Attachment #175237 - Flags: review?(darin)
Attachment #172365 - Flags: review?(darin)
Comment on attachment 175237 [details] [diff] [review]
patch v1.2

>Index: extensions/java/xpcom/XPCOMJavaProxy.java

>+  protected int nativeXPCOMPtr;

it this large enough for native pointers on all platforms?
64-bit platforms included?


>+      if (methodName.equals("hashCode"))  {
>+          return proxyHashCode(aProxy);   
>+      } else if (methodName.equals("equals")) {
>+          return proxyEquals(aProxy, aParams[0]);
>+      } else if (methodName.equals("toString")) {
>+          return proxyToString(aProxy);
>+      } else {

nit: else after return is extraneous.


>+        System.err.println("WARNING: Unhandled XPCOMJavaProxyBase method [" +
>+                           methodName + "]");

nit: might be better to implement some sort of logging system instead.
clearly, these errors wouldn't be shown on windows.  perhaps you could
use NSPR logging?  implement something via JNI that exposes NSPR logging
maybe?	or tie into the console logging service (though that may be 
overkill).  or does System.err get logged to the java console?	do we
expose the java console anymore?  (i'm thinking in terms of Java based
XPCOM components in the browser)


>Index: extensions/java/xpcom/nsJavaInterfaces.cpp

>Index: extensions/java/xpcom/nsJavaWrapper.cpp

>+  if (strncmp("get", aMethodName, 3) == 0) {

is strncmp really correct?  doesn't this match "gett" and "getblah"?
is that a problem?
Attachment #175237 - Flags: review?(darin) → review+
(In reply to comment #16)
> it this large enough for native pointers on all platforms?
> 64-bit platforms included?

Nice catch.  I'll update this when I check it in.

> nit: might be better to implement some sort of logging system instead.

I'll handle this in another bug.

> >+  if (strncmp("get", aMethodName, 3) == 0) {
> 
> is strncmp really correct?  doesn't this match "gett" and "getblah"?
> is that a problem?

Yes, it will match those.  But if it's not a valid method name,
|aIInfo->GetMethodInfoForName(..)| fails, and we carry on.  I guess the only
thing I should be checking for is if |aMethodName| is exactly "get" or "set", in
which case the code would probably crash.
Checked in.  -> FIXED
Status: NEW → RESOLVED
Closed: 20 years ago
Resolution: --- → FIXED
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: