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

[Core] Support kField_EntityReferencesMatchPrefix #566

Closed
feltech opened this issue Aug 11, 2022 · 0 comments · Fixed by #1011
Closed

[Core] Support kField_EntityReferencesMatchPrefix #566

feltech opened this issue Aug 11, 2022 · 0 comments · Fixed by #1011
Assignees
@feltech
Labels
C++

Comments

@feltech
Copy link
Member

feltech commented Aug 11, 2022 •
edited by foundrytom

What

Add support in the C++ implementation of isEntityReferenceString for a prefix-based default via the kField_EntityReferencesMatchPrefix property of the manager's info() dictionary; such that when this key is provided, the Manager wrapper doesn't invoke the corresponding method of the held interface.

Why

When a Python based manager implementation is used, the cost of acquiring the GIL and entering Python in order to invoke the a simple string prefix check can be prohibitive.

We have several docstrings that refer to the kField_EntityReferencesMatchPrefix optimisation/convenience of isEntityReferenceString . However, it is not currently supported, making the docs invalid.

ACs

  • When isEntityReferenceString is called on Manager, if the interfaces info provides the aforementioned key then the implementations corresponding method is not called, and the method returns if the input string begins with the specified prefix.
  • info should only be called once per Manager instantiation`

Notes

Caused by work on #549.

A prerequisite is actually migrating the constant to C++, i.e. #354
@feltech feltech added the C++ label Aug 11, 2022
@feltech feltech mentioned this issue Aug 11, 2022
Closed
8 tasks
@foundrytom foundrytom changed the title [Core] Support (or remove) kField_EntityReferencesMatchPrefix [Core] Support kField_EntityReferencesMatchPrefix Jan 11, 2023
@foundrytom foundrytom mentioned this issue Jan 16, 2023
Closed
feltech added a commit to feltech/OpenAssetIO that referenced this issue Jul 6, 2023
@feltech
[Core] Migrate constants to C++
8b92c32 
Closes OpenAssetIO#998. C++ hosts and managers will need a shared understanding of
common manager info dictionary keys.

In particular, upcoming work will implement the
`isEntityReferenceString` prefix optimisation (OpenAssetIO#566).

Signed-off-by: David Feltell <david.feltell@foundry.com>
feltech added a commit to feltech/OpenAssetIO that referenced this issue Jul 6, 2023
@feltech
[Core] Migrate constants to C++
0863cff 
Closes OpenAssetIO#998. C++ hosts and managers will need a shared understanding of
common manager info dictionary keys.

In particular, upcoming work will implement the
`isEntityReferenceString` prefix optimisation (OpenAssetIO#566).

Signed-off-by: David Feltell <david.feltell@foundry.com>
feltech added a commit to feltech/OpenAssetIO that referenced this issue Jul 6, 2023
@feltech
[Core] Migrate constants to C++
a3082a4 
Closes OpenAssetIO#998. C++ hosts and managers will need a shared understanding of
common manager info dictionary keys.

In particular, upcoming work will implement the
`isEntityReferenceString` prefix optimisation (OpenAssetIO#566).

Signed-off-by: David Feltell <david.feltell@foundry.com>
feltech added a commit to feltech/OpenAssetIO that referenced this issue Jul 6, 2023
@feltech
[Core] Migrate constants to C++
21fe32a 
Closes OpenAssetIO#998. C++ hosts and managers will need a shared understanding of
common manager info dictionary keys.

In particular, upcoming work will implement the
`isEntityReferenceString` prefix optimisation (OpenAssetIO#566).

Signed-off-by: David Feltell <david.feltell@foundry.com>
@feltech feltech self-assigned this Jul 6, 2023
feltech added a commit to feltech/OpenAssetIO that referenced this issue Jul 6, 2023
@feltech
[Core] isEntityReferenceString prefix optimisation
a3ba162 
Closes OpenAssetIO#566. When a Python based manager implementation is used, the
cost of acquiring the GIL and entering Python in order to invoke a
simple string prefix check can be prohibitive.

We have several docstrings that refer to the
`kInfoKey_EntityReferencesMatchPrefix` optimisation/convenience of
`isEntityReferenceString`. However, it was not supported, making the
docs invalid.

So add support for using this optional optimisation feature.

Signed-off-by: David Feltell <david.feltell@foundry.com>
@feltech feltech mentioned this issue Jul 6, 2023
Merged
2 tasks
feltech added a commit to feltech/OpenAssetIO that referenced this issue Jul 7, 2023
@feltech
[Core] Migrate constants to C++
6a2093b 
Closes OpenAssetIO#998. C++ hosts and managers will need a shared understanding of
common manager info dictionary keys.

In particular, upcoming work will implement the
`isEntityReferenceString` prefix optimisation (OpenAssetIO#566).

Signed-off-by: David Feltell <david.feltell@foundry.com>
feltech added a commit to feltech/OpenAssetIO that referenced this issue Jul 7, 2023
@feltech
[Core] isEntityReferenceString prefix optimisation
84f8e3c 
Closes OpenAssetIO#566. When a Python based manager implementation is used, the
cost of acquiring the GIL and entering Python in order to invoke a
simple string prefix check can be prohibitive.

We have several docstrings that refer to the
`kInfoKey_EntityReferencesMatchPrefix` optimisation/convenience of
`isEntityReferenceString`. However, it was not supported, making the
docs invalid.

So add support for using this optional optimisation feature.

Signed-off-by: David Feltell <david.feltell@foundry.com>
feltech added a commit to feltech/OpenAssetIO that referenced this issue Jul 7, 2023
@feltech
[Docs] Improve manager entity ref prefix usage docs
58dfcea 
Part of OpenAssetIO#566. Add a note that manager plugin authors should consider
making use of the `kInfoKey_EntityReferencesMatchPrefix` feature, now
that it actually works.

Signed-off-by: David Feltell <david.feltell@foundry.com>
feltech added a commit to feltech/OpenAssetIO that referenced this issue Jul 12, 2023
@feltech
[Core] isEntityReferenceString prefix optimisation
77daac6 
Closes OpenAssetIO#566. When a Python based manager implementation is used, the
cost of acquiring the GIL and entering Python in order to invoke a
simple string prefix check can be prohibitive.

We have several docstrings that refer to the
`kInfoKey_EntityReferencesMatchPrefix` optimisation/convenience of
`isEntityReferenceString`. However, it was not supported, making the
docs invalid.

So add support for using this optional optimisation feature.

Signed-off-by: David Feltell <david.feltell@foundry.com>
feltech added a commit to feltech/OpenAssetIO that referenced this issue Jul 12, 2023
@feltech
[Docs] Improve manager entity ref prefix usage docs
f311be2 
Part of OpenAssetIO#566. Add a note that manager plugin authors should consider
making use of the `kInfoKey_EntityReferencesMatchPrefix` feature, now
that it actually works.

Signed-off-by: David Feltell <david.feltell@foundry.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@feltech feltech

Labels
C++
Projects
OpenAssetIO Development
Status: Done
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

isEntityReferenceString prefix optimisation feltech/OpenAssetIO
1 participant
@feltech