Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Extension #890

Open
wants to merge 13 commits into
base: master
Choose a base branch
from
Open

Extension #890

wants to merge 13 commits into from

Conversation

Thrameos
Copy link
Contributor

This is a work in progress PR for extending classes within Python.

@lgtm-com
Copy link

lgtm-com bot commented Nov 15, 2020

This pull request introduces 23 alerts when merging 7fbc10b into 7256261 - view on LGTM.com

new alerts:

  • 18 for Dereferenced variable may be null
  • 2 for Container contents are never accessed
  • 1 for Useless comparison test
  • 1 for Dereferenced variable is always null
  • 1 for Array index out of bounds

@Thrameos
Copy link
Contributor Author

Thrameos commented Nov 16, 2020

Making progress, but still deep technical issues.

  • We need to be able to call super from the constructor (__init__) and no place else.
  • We need to make sure base __init__ can only be called once.
  • We need to make sure that initial assignments are to "None" or a primitive type.
  • We need to be able keep any methods or fields from being called before the init.
  • We need to grant privilege access to private and protected methods within these implementations. This privilege should not be passed to copies or methods called from implementation if possible.
  • We need to handle exceptions.
  • We need to add @JThrows to define exceptions.
  • We need to aggressively catch errors before the classloader.
  • We need a special class loader for these types.

It is still not clear if we can invokespecial from JNI.

@lgtm-com
Copy link

lgtm-com bot commented Nov 16, 2020

This pull request introduces 20 alerts when merging 472b300 into 7256261 - view on LGTM.com

new alerts:

  • 18 for Dereferenced variable may be null
  • 1 for Useless comparison test
  • 1 for Array index out of bounds

@lgtm-com
Copy link

lgtm-com bot commented Nov 16, 2020

This pull request introduces 20 alerts when merging 3855389 into 39ae320 - view on LGTM.com

new alerts:

  • 18 for Dereferenced variable may be null
  • 1 for Useless comparison test
  • 1 for Array index out of bounds

@Thrameos
Copy link
Contributor Author

I added documentation on the initial guess of where I think the Java extension system will be headed. Any input on the direction would be appreciate.

https://github.com/jpype-project/jpype/pull/890/files#diff-f55f8f25b97e5081e65a0e0db39f6c98ed08d4deea664b3137b9ce3ea52c1245

@lgtm-com
Copy link

lgtm-com bot commented Nov 16, 2020

This pull request introduces 20 alerts when merging c3facdd into 39ae320 - view on LGTM.com

new alerts:

  • 18 for Dereferenced variable may be null
  • 1 for Useless comparison test
  • 1 for Array index out of bounds

@marscher
Copy link
Member

This lists reads like we need a complete re-implementation of essential Java features. Ain't that a bit of an overkill for this feature? What do you think will be the expected performance, when cross-calling between the languages during handling extended classes?
I guess you have already a path to the truth lying around, and I want to be enlightened as well :)

@Thrameos
Copy link
Contributor Author

Extending a java class remains a requested feature. Some APIs in Java require a class to be extended. We can achieve this by writing a custom Java class and a stub interface that then has to loaded with the user code. But that is a real pain.

This is the alternative where the user declares a Java extension in Python which is "compiled" at runtime. The performance of the compile is not great but you should not do it often. The actual call cost is the same as the existing Java JProxy code. Everything must be boxed. Each gets a Java handle for Python to use. Then Python gets called. The return has to be boxed again. Then we return to Java space. So lets assume 5 arguments with 2 primitives. That is 2 box objects then 5 python objects, the one java object on the return. We also have the tuple for the call transfer. So cost is 9 object creation. Compare this to one python method call. One object for the bind. One for the tuple. Several more if keyword args. So a Java to Python call is a slightly more heavy than a Python call and much more than a native Java call.

@Thrameos
Copy link
Contributor Author

Setting up the privilege flag is challenging. I need to be able to check the flag whenever a private method is called and verify the flag, but there is no space in the object currently for it (because there memory layout.)

Options are:

  • Create a second dynamic type for the class which holds the private fields and methods. The assign the first argument to it rather than the actual type of the object. Only extension objects require this second type but it does require restructuring of class creation functions to generate 2 classes rather than one.
    This only allows access to private methods, and fields rather protected as the tree is unlike to handle a second protection branch. The downside is that anything can capture this object and then get privileged access. Not really what I intend.
  • use the object dictionary to set the privilege flag then then the flag whenever a private method or field is called. This has the downside that it forces a dictionary on every object and costs a lot of time in lookups. Of course this is only used when privileged object is created so the memory and time would be reasonable.
  • add another storage spot in the JPValue for the privilege state as flags. That is a lot of memory for a new feature given every object will need it. But this would be O(1). There may be other features that will require a flags field on JPValue.
  • Hack a space for it in Python structures. This would be non portable.
  • fail to enforce privilege. Well this has significant downsides in terms of allowing private fields or methods to be called.
  • Create a special "This" type that points to another object and unpacks to the original, redirects method and field lookups to the original. This would need some work on the type matching system as it needs to be recognized as the original object when used as a call. The downside would be the type of the object when reporting would not be the type expected. It can also be captured which breaks down the privilege model.
  • Force privilege access to go through a special member such as "private" when can delegate the binding. This has most of the downsides of a special class for the call.
  • Use a double redirection scheme which places the privilege instead of in the object but on the stack. In this case we call an function which adds to the Python call stack the emitter. When we want to check for privileged access we poll the stack to see if we were called from the privileged method. This would work so long as one method does not call another. If it does then the privilege is lost. This has the advantage that privilege is automatically lost when the method exits regardless of the reference or the path taken. This has the upside that privilege is not granted to a method which is called from the internal.

@Thrameos Thrameos added this to In progress in JPype 2.0 via automation Nov 26, 2020
@Thrameos Thrameos added the enhancement Improvement in capability planned for future release label Nov 26, 2020
@Thrameos Thrameos self-assigned this Nov 26, 2020
@Thrameos Thrameos added this to the JPype 2.0.0 milestone Nov 26, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Improvement in capability planned for future release
Projects
JPype 2.0
  
In progress
Development

Successfully merging this pull request may close these issues.

None yet

2 participants