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

Refactor refobj #441

Draft
wants to merge 18 commits into
base: main
Choose a base branch
from
Draft

Refactor refobj #441

wants to merge 18 commits into from

Conversation

bbartley
Copy link
Contributor

This experimental feature branch aims to eliminate costly lookup operations. It was motivated by the fact that LabOP protocols, even relatively simple ones, start to bog down during protocol execution. Profiling showed that execution was getting bogged down by lookups and finds that take a long time to traverse over the Document tree. As the protocol executes, new objects are dynamically generated, making these document traversals ever more costly. Moreover, a conventional caching approach (e.g., Python functools) was ineffective, as the lookups are predominantly performed on nascent, uncached objects.

  • ReferencedObject attributes have been refactored so that a ReferencedObject attribute will always return an object, not a URI
  • Thus ReferencedObject attributes always contain direct pointers to Python objects in memory; no document traversal is required to dereference the object
  • When a ReferencedObject refers to an external object not currently contained in the Document, a stub SBOLObject is used instead
  • Implements rudimentary reference counter/garbage collection (for lack of better words)
  • Almost maintains reverse compatibility (almost)

@jakebeal
Copy link
Contributor

I'm quite sympathetic to this idea, having experienced the significant time costs myself.

I think that my key concern ends up being around the idea of the implementation of out-of-document objects via anonymous stubs.

What would you think of changing from anonymous stubs to objects of a subclass with a name like MissingObject or OutOfDocument? (I'm not sure whether it should be a subclass of TopLevel or of SBOLObject). Having a designated subclass would let a program that's traversing a document be able to know explicitly and positively that it's encountering an out-of-document link, and take actions like trying to resolve the link or ignoring the link.

@tcmitchell
Copy link
Collaborator

@bbartley we should merge main into this PR because two other PRs have been merged. One of the other PRs fixes the read-the-docs issue that is causing this PR's build to break.
Would you like me to do the merge? I don't want to step on toes if you're actively developing.

@bbartley
Copy link
Contributor Author

@tcmitchell go for it! this PR is at a stable point and the only thing failing is the read the docs. it would be great to see it go green

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.

3 participants