GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — Better lazy resolution by ctrueden · Pull Request #91 · scijava/scijava-common

Conversation

@ctrueden

@ctrueden ctrueden commented Jul 1, 2014

Copy link
Copy Markdown
Member

MERGE ONLY IN CONCERT WITH THE OTHER 3.0.0 BRANCHES.

This improves the ObjectIndex lazy object resolution feature to only resolve objects as needed based on the type. Previously, as soon as get(Class) or getAll() was called, all pending objects would be resolved. Now we call the new LazyObjects#getType() method to see if the type of the Collection is even worth bothering about.

See also issue #90.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@ctrueden ctrueden mentioned this pull request Jul 1, 2014
Limited Context tries to instantiate invalid plugins #90
Open
@ctrueden

ctrueden commented Jul 2, 2014

Copy link
Copy Markdown
Member Author

I have come to the conclusion that this approach is simply not good enough in all scenarios. We need to add an ObjectIndex#addLater(LazyObjects, Class) method signature that lets you explicitly specify the type of the objects which will be resolved. Otherwise, the behavior will never be good enough.

I will work on that as soon as I can...

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@ctrueden

ctrueden commented Jul 7, 2014

Copy link
Copy Markdown
Member Author

I reworked the branch to add a getType() method to LazyObjects which returns the type of the to-be-resolved objects. This information is then used to decide whether to resolve the objects when ObjectIndex#get(Class) is called.

It is working fine, but is a breaking change from the SJC 2.x.x API, so bumps the snapshot version to 3.0.0. It should be merged only in concert with other breaking SJC branches, particularly robust-io (which is not yet complete).

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@ctrueden ctrueden added this to the 3.0.0 milestone Jul 25, 2014
ctrueden and others added 7 commits May 12, 2015 16:47
@ctrueden
ObjectIndexTest: improve getAll() test coverage
f4dce23
Specifically, we test that lazy object resolution works in conjunction
with a call to getAll(), to ensure that pending objects get resolved.
@ctrueden
AbstractSingletonService: use more specific type
5ef3f68
The LazyObjects instance will definitely return a collection of objects
of type PT, not just Object. This distinction is important now that the
ObjectIndex is more selective about which pending objects it resolves.
@ctrueden
LazyObjects: add getType() method
8bbb47a
This breaks backwards compatibility, so bumps the development snapshot
version to 3.0.0.

The rationale is to make it possible for the ObjectIndex to better
discriminate which objects should be resolved for a particular call to
ObjectIndex#get(Class). The current behavior is to resolve _all_ pending
objects with _any_ call to get(Class), which is generally suboptimal.
@ctrueden
ObjectIndex: only resolve relevant pending objects
29ff80b
If a pending object will not be of a type compatible with the
requested type, then skip resolution of that pending object.

This improves performance of the get(Class) and getAll() methods in
cases where there are a large number of pending objects in the index.

It also helps to eliminate difficulties like those in issue #90.
@ctrueden
ObjectIndexTest: add a lazy object resolution test
843f863
This tests that the addLater method works in conjunction with calls to
get(Class). The logic is nontrivial because we only want to resolve
pending objects when the types are compatible with the requested one.
@hinerm @ctrueden
AbstractSingletonService: fix helper method
05b1537
The filterInstances method should not alter the return type of the input
list. The purpose of that method is to potentially remove some elements
from the input list -- nothing more -- so it should still return a
List<PT>, not something else.
@hinerm @ctrueden
Lean on object service for singleton retrieval
d6a7eba
Instead of maintaining our own list of singletons, we should be using
the ObjectService.
@ctrueden
ctrueden force-pushed the better-lazy-resolution branch from cbc827c to d6a7eba Compare May 12, 2015 22:02
@ctrueden

ctrueden commented May 12, 2015

Copy link
Copy Markdown
Member Author

While rebasing this branch, I noticed a problem with AbstractSingletonService as written. It now has a hard map of instances (due to 1b245c1). I think we should reconsider this—perhaps the cache should be weak and lazy instead. And unit tests, unit tests.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@ctrueden
ctrueden force-pushed the master branch 3 times, most recently from 53b6733 to 3dc99c9 Compare November 7, 2023 18:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

3.0.0

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

2 participants

@ctrueden @hinerm
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402520 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.