Skip to content

feat: add capability to skip loading the AST from storage#1101

Merged
ava-fred merged 1 commit intodsldevkit:masterfrom
ava-fred:work.250228
Mar 10, 2025
Merged

feat: add capability to skip loading the AST from storage#1101
ava-fred merged 1 commit intodsldevkit:masterfrom
ava-fred:work.250228

Conversation

@ava-fred
Copy link
Copy Markdown
Collaborator

@ava-fred ava-fred commented Mar 7, 2025

This commit introduces a new ResourceSetOption, using which it is possible to skip loading the AST when loading a resource from binary storage.

resourceSet.getLoadOptions().put(INSTALL_DERIVED_STATE, installDerivedState);
}

public static boolean skipAST(final @NonNull ResourceSet resourceSet) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

and change the name and default here as well

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

What would I need to change it to?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

maybeSkipModel?

return object != null && (boolean) object; // default is false
}

public static void setSkipAST(final @NonNull ResourceSet resourceSet, final @Nullable Boolean installDerivedState) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

the name installDerivedState needs to be changed, right?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I don't understand why it needs to be changed, please explain.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

the second parameter installDerivedState should be named installAST, shouldn't it? Or with the naming I suggest, installModel and the method called setSkipModel

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Yes it should. I have done the rename you suggested and used "model" instead of "AST".

BasicEList<InternalEObject> internalEObjectList = new BasicEList<InternalEObject>(); // NOPMD LooseCoupling
internalEObjectList.setData(1, values);
@SuppressWarnings("unchecked")
InternalEList<InternalEObject> internalEObjects = (InternalEList<InternalEObject>) (InternalEList<?>) resource.getContents();
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

what about adding subclassed BasicEList which throws an UnsoportedOperationException in all operations? That way we are sure that we are not getting wrong results because we access a partially loaded resource.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

No, that's not going to work: this list is returned from Resource.getContents(), the inference container is contained in it

This commit introduces a new ResourceSetOption, using which it is
possible to skip loading the AST when loading a resource from binary
storage.
@ava-fred ava-fred merged commit 73f5e43 into dsldevkit:master Mar 10, 2025
2 checks passed
@ava-fred ava-fred deleted the work.250228 branch March 10, 2025 14:49
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