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: