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.

Sign up for GitHub

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

Jump to bottom

refractored add references functions to greatly improve xml import speed. #1111

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from
Draft

refractored add references functions to greatly improve xml import speed. #1111

wants to merge 3 commits into from

Conversation

jcbastosportela
Copy link
Contributor

With this changes I was able to import a set of XMLs in about 35s in my machine. Without this changes the same set of inf models won't finish importing even after 5 minutes!

oroulet
oroulet requested changes Nov 3, 2022
View reviewed changes
asyncua/server/address_space.py
NODE_ID_HAS_PROPERTY = ua.NodeId(ua.ObjectIds.HasProperty)
NODE_ID_NULL = ua.NodeId(ua.ObjectIds.Null)
NODE_ID_HASSUBTYPE = ua.NodeId(ua.ObjectIds.HasSubtype)
NODE_ID_DEFAULT = ua.NodeId()
Copy link
Member

Choose a reason for hiding this comment

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

Does that help?
I have been considering doing that in main library but i thought someone concluded it does not help much

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, from my tests this helps.

asyncua/ua/uaprotocol_auto.py Outdated
@@ -5906,6 +5906,8 @@ def NodeClass(self):
def NodeClass(self, val):
self.NodeClass_ = val

def __hash__(self):
return hash((self.ReferenceTypeId, self.NodeId, self.IsForward))
Copy link
Member

Choose a reason for hiding this comment

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

You cannot modify an autogenerated file,
Modify the generating file!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Didn't notice this was generated, sorry, will look into the generator

asyncua/ua/uatypes.py
:ivar NamespaceUri:
:vartype NamespaceUri: String
:ivar ServerIndex:
:vartype ServerIndex: Int
Copy link
Member

Choose a reason for hiding this comment

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

Why do you remove documentation?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This doc was redundant with the Args: above, also it does not match the dataclass argument list

Copy link
Member

Choose a reason for hiding this comment

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

This is a special format to generate documentation automatically. better to update it than remove it

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will do.
Thought the format above was supported (i think that is google style, also the one I use)

asyncua/ua/uatypes.py
@@ -380,7 +380,7 @@ class NodeIdType(IntEnum):
_NodeIdType = NodeIdType # ugly hack


@dataclass(frozen=True, eq=False, order=False)
@dataclass(slots=True, frozen=True, eq=False, order=False)
Copy link
Member

Choose a reason for hiding this comment

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

I remember we considered using slot instead of our custon frozen thing. We had some reasons for not using it...not remembering what...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Slots help, but it was not the biggest improvement. Will revert this

@jcbastosportela
Copy link
Contributor Author

I tried to run the update_ua_nodeset.py and I keep getting errors in multiple places.
Some of the issues I faced:

  • update_ua_nodeset.py : 62 fails because of the permissions of .git folder in windows (worked around by manually deleting and skipping those steps)
  • update_ua_nodeset.py : 64 fails because for current master branch of opc ua that file no longer exists; targeted branch (workaround by skipping that rm)
  • generate_event_object.py : 85 fails because seems that master opc ua has some NodeIds not implemented by the generator. Error:
...
  File "C:\Users\jportela\AppData\Local\Programs\Python\Python310\lib\runpy.py", line 86, in _run_code
    exec(code, run_globals)
  File "c:\Sandbox\opcua-asyncio\schemas\generate_event_objects.py", line 158, in <module>
    ecg.generate_events_code(model)
  File "c:\Sandbox\opcua-asyncio\schemas\generate_event_objects.py", line 138, in generate_events_code
    self.generate_event_class(event)
  File "c:\Sandbox\opcua-asyncio\schemas\generate_event_objects.py", line 116, in generate_event_class
    self.add_properties_and_variables(event)
  File "c:\Sandbox\opcua-asyncio\schemas\generate_event_objects.py", line 62, in add_properties_and_variables
    ref.refBrowseName, self.get_value(ref),
  File "c:\Sandbox\opcua-asyncio\schemas\generate_event_objects.py", line 85, in get_value
    str(ob_ids.ObjectIdNames[int(str(reference.refId).split("=")[1])]).split("_")[0])
KeyError: 31771
  • when giving "-b v1.04" fails with:
['git', 'clone', '--depth=1', 'https://github.com/OPCFoundation/UA-Nodeset.git', 'UA-Nodeset-master', 'v1.04']
fatal: Too many arguments.

Other notes: the generator script assumes that the script is running with cwd on the same dir of the update_ua_nodeset.py, maybe this should be fixed as I noticed that later.

In the end I quit trying to run it :(
Because the most important changes of this PR needs to be done on the generator and I cannot run it I am moving this PR to draft until I get more time do dedicate to this.

@jcbastosportela jcbastosportela marked this pull request as draft November 9, 2022 13:00
@schroeder-
Copy link
Contributor

schroeder- commented Nov 9, 2022 •
edited
Loading

update_ua_nodeset.py : 62 fails because of the permissions of .git folder in windows (worked around by manually deleting and skipping those steps)

This is caused by visual studio code. There is no workaround only closing visual studio code.

update_ua_nodeset.py : 64 fails because for current master branch of opc ua that file no longer exists; targeted branch (workaround by skipping that rm)

Last time i tried it work. Will try soon.

generate_event_object.py : 85 fails because seems that master opc ua has some NodeIds not implemented by the generator. Error:

Noticed it last time too. But just reverted to the current used nodeset UA1.05.01.

The easiest way currently is to check out the nodeset your self via git. Then execute all the scripts:

py -3 generate_address_space.py
py -3 generate_event_objects.py
py -3 generate_ids.py
py -3 generate_protocol_python.py
py -3 generate_model_event.py
py -3 generate_event_objects.py
py -3 generate_statuscode.py
py -3 generate_uaerrors.py

Also be carefull if you make errors generating schemas you need to revert the changes via git. Because some generating scripts use generated code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@oroulet oroulet oroulet requested changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@jcbastosportela @schroeder- @oroulet @jcbportela