8000 Injection by ID by migus88 · Pull Request #764 · hadashiA/VContainer · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Injection by ID #764

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
Draft
wants to merge 22 commits into
base: master
Choose a base branch
from
Draft

Injection by ID #764

wants to merge 22 commits into from

Conversation

migus88
Copy link
@migus88 migus88 commented Apr 11, 2025 •
edited
Loading

WORK IN PROGRESS

This PR is work in progress. While the logic is fully complete, existing tests are passing and new tests are in place, before opening it for a full review, I'd like to give it a better test drive in my own project - to see if I missed something.
Before I open this PR for a review, I might fix some bugs, improve the readability of the changes and add more tests coverage, but overall the core logic shouldn't change, so feel free getting familiar with the change and giving me your suggestions.

Description

As a long time fan of this DI framework, I use it constantly, but I was always missing one feature that I got really used to in Zenject - injection by ID.

I know we can use Factories and that's what I did up until recently, but since I had a little spare time, I wanted to implement the feature in VContainer as well.

My aim was to not introduce any regression and to do it as performant as possible.

Usage:

// Registration:
builder.Register<IWeapon, Sword>(Lifetime.Singleton).WithId(SomeEnum.Primary);
builder.Register<IWeapon, Bow>(Lifetime.Singleton).WithId("secondary");
builder.Register<IWeapon, Knife>(Lifetime.Singleton).WithId(3);

// Container Resolution:
var primaryWeapon = container.Resolve<IWeapon>(SomeEnum.Primary);
var secondaryWeapon = container.Resolve<IWeapon>("secondary");

// Constructor Injection:
public SomeClass(
        [Inject(SomeEnum.Primary)] IWeapon primaryWeapon,
        [Inject("secondary")] IWeapon secondaryWeapon)
{
    this.primaryWeapon = primaryWeapon;
    this.secondaryWeapon = secondaryWeapon;
}

// Method Injection:
[Inject]
public void Construct(
        [Inject(SomeEnum.Primary)] IWeapon primaryWeapon,
        [Inject("secondary")] IWeapon secondaryWeapon)
{
    PrimaryWeapon = primaryWeapon;
    SecondaryWeapon = secondaryWeapon;
}

// Property and Field Injection
[Inject(SomeEnum.Primary)]  public IWeapon PrimaryWeapon;
[Inject("secondary")] public IWeapon SecondaryWeapon { get; set; }

Benchmarks

Implementation Change

In order to support resolution by ID, I had to turn the FixedTypeKeyHashTable class into FixedTypeObjectKeyHashtable.
I've created a quick benchmark (using BenchmarkDotNet) for the class (see the benchmark here) and here are the results for it:

Method DictionarySize Mean Error StdDev Allocated
'Dictionary<Type, object>' 100 77.33 ns 1.567 ns 3.057 ns -
'Dictionary<(Type, object), object>' 100 114.69 ns 0.902 ns 0.843 ns -
FixedTypeKeyHashtable 100 22.63 ns 0.251 ns 0.223 ns -
FixedTypeObjectKeyHashtable 100 27.26 ns 0.543 ns 0.557 ns -
'Dictionary<Type, object>' 1000 76.30 ns 0.187 ns 0.156 ns -
'Dictionary<(Type, object), object>' 1000 137.71 ns 1.099 ns 0.918 ns -
FixedTypeKeyHashtable 1000 22.76 ns 0.250 ns 0.195 ns -
FixedTypeObjectKeyHashtable 1000 26.14 ns 0.075 ns 0.062 ns -
'Dictionary<Type, object>' 10000 76.80 ns 0.510 ns 0.477 ns -
'Dictionary<(Type, object), object>' 10000 104.48 ns 1.152 ns 1.021 ns -
FixedTypeKeyHashtable 10000 22.94 ns 0.476 ns 0.445 ns -
FixedTypeObjectKeyHashtable 10000 26.46 ns 0.162 ns 0.135 ns -

We can see that the performance of FixedTypeObjectKeyHashtable is pretty comperable to FixedTypeKeyHashtable. Slight slowdown is expected due to hash combination.

Built in benchmarks

I've fixed and ran the built in benchmarks both on master and on my branch. There is a slight performance degradation during container building due to a bit more complex registration. Resolving is unchanged (the benchmarks show that in my branch the resolving is a bit faster, but I think it's inside a margin of error).
Benchmark Comparison - GC
Benchmark Comparison - Millisecond_Median

@vercel Vercel
Copy link
vercel bot commented Apr 11, 2025 •
edited
8000 Loading

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
vcontainer ✅ Ready (Inspect) Visit Preview 💬 Add feedback Apr 30, 2025 7:49am

@migus88 migus88 marked this pull request as draft April 13, 2025 05:23
@softshadedev
Copy link

@migus88 I encountered a problem, multiple registration for single interface does not work as expected. Inject IEnumerable will always contain zero elements if the method with ID was used.

@migus88
Copy link
Author
migus88 commented Apr 29, 2025

@softshadedev thanks, I will take a look at it!
Probably this scenario isn't covered by unit tests - will add them as well.

I also want to see how I can move the registration with ID to use the FixedTypeKeyHashtable instead of dictionary and to merge the functionalities. This should also solve the issue you've mentioned.

@migus88
Copy link
Author
migus88 commented Apr 30, 2025

@softshadedev I've merged the handling of non-id registrations with id registrations and added a unit test to make sure that collections returned as expected no matter how the dependency is registered. Give it a try.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@migus88 @softshadedev
0