Skip to content

Type improvements#5

Draft
fmagin wants to merge 2 commits into
justfoxing:masterfrom
fmagin:type_improvements
Draft

Type improvements#5
fmagin wants to merge 2 commits into
justfoxing:masterfrom
fmagin:type_improvements

Conversation

@fmagin
Copy link
Copy Markdown
Contributor

@fmagin fmagin commented May 5, 2020

Extracted from #4

@fmagin fmagin marked this pull request as draft May 5, 2020 11:15
Comment thread jfx_bridge/bridge.py
serialized_dict = {TYPE: BRIDGED, VALUE: data._bridge_handle}
elif isinstance(data, type(None)):
serialized_dict = {TYPE: NONE}
elif hasattr(data, "typeName"):
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typeName is only a Jython thing, this won't work for other environments

Comment thread jfx_bridge/bridge.py
try:
# For objects this has not just the class, but the full module path
# e.g. "ghidra.program.database.ProgramDB" for currentProgram instead of just "ProgramDB"
_type = type(self.local_obj).typeName
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As mentioned, only true for Jython. I think it might make more sense for your usecase to explicitly query for the typeName object in your code, instead of embedding it here.

Comment thread jfx_bridge/bridge.py
return {HANDLE: self.handle, TYPE: _type, ATTRS: self.attrs, REPR: repr(self.local_obj)}

if hasattr(self.local_obj, "typeName"):
_name = self.local_obj.typeName
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why add _name, what do you get that's different from _type?

Copy link
Copy Markdown
Contributor Author

@fmagin fmagin May 7, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For a Class like ghidra.GhidraApplicationLayout _type will be something like JavaClass and __name__ is GhidraApplicationLayout. But I need the ghidra.GhidraApplicationLayout which is in .typeName for a Class object

@fmagin
Copy link
Copy Markdown
Contributor Author

fmagin commented May 7, 2020

Ok, seems like this doesn't work as I thought overall.

What would solve my problem is maybe some way to register functions for attributes that retrieve the attribute on object creation so I don't have to waste bridge traversals when I expect that I will need this for many objects/classes anyway and it is just a string.

Batch Get somewhat alleviates this, but I think the cost of adding a few strings for every object creation is basically zero, while having to traverse the bridge later (even for a batch call) is around 10-20ms of latency. I think if it is really just one batch call per object introspection it is acceptable, but this would also require that accessing __class__ is properly faked by the bridge (it currently isn't but I am also looking into that). Otherwise it is already 10ms to get __class__, then 10ms to get typeName. This tends to quickly add up into noticeable latency, especially if it is fore autocompletion that times out after ~300ms. I will play around with my code some more and keep the feedback from this in mind, and see what would be other good approaches. I think Jython environments will require some extra tricks on the server side to handle properly, but I am not yet sure how to accommodate that without overcomplicating the code.

@justfoxing justfoxing force-pushed the master branch 4 times, most recently from e1231a9 to a1317b6 Compare July 14, 2020 05:52
@justfoxing justfoxing force-pushed the master branch 3 times, most recently from a298e1b to 0658bd4 Compare February 14, 2022 10:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants