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

Improve locking on container.nodes #24

Closed
anuptalwalkar opened this issue Apr 26, 2017 · 4 comments
Closed

Improve locking on container.nodes #24

anuptalwalkar opened this issue Apr 26, 2017 · 4 comments

Comments

@anuptalwalkar
Copy link
Contributor

container.nodes map is being used in exported functions that are explicitly locked. We need to improve container locking once #21 is landed. Invoke is taking too long, we don't want to lock the entire dig container.

@glibsm
Copy link
Collaborator

glibsm commented Apr 26, 2017

Define "too long" and what would make it not "too long".
Do you have a target in mind for how long invoke should take?

@anuptalwalkar
Copy link
Contributor Author

ok, "too long" may be a loose wording, but essentially we are handing over the container lock to Invoked function for the duration of its execution (resolving dependency will involve executing constructors as well). I am proposing to use locking only when c.nodes is accessed for read/writes instead of locking for entire exported function for Provide, Resolve and Invoke.

@glibsm
Copy link
Collaborator

glibsm commented Apr 26, 2017

I suggest that we write a benchmark, and consider that this is only happening at service startup. That should give us some ammo to talk about what's "too long".

Locking entire Invoke/Resolve should not be that bad in favor of consistency, but I may be wrong.

This is not something that happens 20k times a second, but rather 10 times on bootup.

@glibsm
Copy link
Collaborator

glibsm commented May 17, 2017

Closing in light of #50
We're going to keep non-concurrent and push those decisions up the stack to whomever is using dig, be that framework or a single user

@glibsm glibsm closed this as completed May 17, 2017
hbdf pushed a commit to hbdf/dig that referenced this issue Jul 14, 2022
hbdf pushed a commit to hbdf/dig that referenced this issue Jul 14, 2022
* Remove traffic reporter/tracker

In favor of modules emitting metrics natively.

GFM-108
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants