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

~GrpcMuxWatchImpl() uses watches_.remove(this) which traverses the list. #15528

Closed
efimki opened this issue Mar 16, 2021 · 7 comments · Fixed by #15534
Closed

~GrpcMuxWatchImpl() uses watches_.remove(this) which traverses the list. #15528

efimki opened this issue Mar 16, 2021 · 7 comments · Fixed by #15534
Assignees
Labels
triage Issue requires triage

Comments

@efimki
Copy link
Contributor

efimki commented Mar 16, 2021

Title: ~GrpcMuxWatchImpl() uses watches_.remove(this) which traverses the list.

Description:

~GrpcMuxWatchImpl() uses std::list::remove() to remove itself from watches_ list.
This traverses the list and takes O(N) for each destruction.
We could speed things up to O(1) if GrpcMuxWatchImpl would keep a list iterator for itself and used std::list::erase() instead.

@efimki efimki added the triage Issue requires triage label Mar 16, 2021
@stevenzzzz
Copy link
Contributor

@stevenzzzz
Copy link
Contributor

let's do similar trick as is done in #11751

@stevenzzzz
Copy link
Contributor

stevenzzzz commented Mar 16, 2021

cc @htuch

@stevenzzzz
Copy link
Contributor

/cc @htuch

@chaoqin-li1123
Copy link
Member

Yeah, I think we can store the list iterator and use it for constant time removal.

@htuch
Copy link
Member

htuch commented Mar 17, 2021

Sure, makes sense.

@mk46
Copy link
Contributor

mk46 commented Mar 17, 2021

I am taking this. PTAL to linked PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
triage Issue requires triage
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants