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

ST6RI-684 (update) Implement invocation delegates for operations #555

Merged
merged 1 commit into from
Apr 6, 2024

Conversation

seidewitz
Copy link
Member

Certain OCL operations in the specification, related to computing the imported memberships of a Namespace and the inherited memberships of a Type, have particularly complicated implementations. Previously, the implementation of these operations was decomposed into a number of additional methods in the NamespaceImpl and TypeImpl classes (and certain subclasses of those), and some of those methods called each other (rather than just calling on the methods that directly implemented the OCL operations). Part of the update in PR #554 was to move all these operations out of the Impl classes and into the various invocation delegates implementing the specified OCL operations.

However, in the implementation as approved for PR #554, the cross-calls of methods defined in different invocation delegate classes required the dynamic creation of delegate instances using the reflective OperationInvocationDelegateFactory mechanism, in order to properly resolve method overrides. Unfortunately, this turned out to significantly degrade performance of parsing and validation.

This PR implements an alternate approach in which the additional methods in question are moved out of the invocation delegates and into corresponding adapter classes. This has the disadvantage that it couples certain invocation delegates to the adapter implementations. Nevertheless, there is a significant benefit, since overridden methods in adapter subtypes are resolved using normal Java dynamic dispatching. As a result, the performance goes back to being essentially the same as it as it was before the implementation of PR #554.

- This was necessary because the delegate-based implementation caused
too big of a performance degredation.
@seidewitz
Copy link
Member Author

PR #556 proposes an alternative solution to the problem addressed in this PR which is a smaller change to the code already adopted in PR #554 than is proposed in this PR. However, the adapter-based approach proposed here is arguable a simpler more direct approach to handling the additional methods in question than that originally introduced in PR #554, and it is consistent with some previous cases of moving additional state and methods from Impl classes to adapter classes. It also seems to perform slightly better than the approach of PR #556.

So, the decision has been made to accept this PR rather PR #556, though the ST6RI-486-2 branch of #556 will be retained for possible future reference.

@seidewitz seidewitz merged commit 84996a6 into master Apr 6, 2024
2 checks passed
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.

1 participant