Skip to content

[http_server] Simplify DRT node and table #132

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Draft
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

cong1920
Copy link
Contributor

@cong1920 cong1920 commented Dec 31, 2024

When I see the std::vecotr<std::shared_ptr<drt_node>> and std::vector<std::shared_ptr<std::string>> as the drt_node and dynamic_routing_table's members respectively, I cannot help wondering why the shared_ptr<T> are needed here. Later I realized the instance of the DRT table is allowed to copy therefore we need to keep its objects being shared across all instances. Somehow I still think it is a waste to have shared_ptr<T> manage each small object.

This PR includes

  • unordered_set<string> to keep the strings other than std::vector<std::shared_ptr<std::string>>. unordered_set<string> is more straightforward and able to get rid of redundant duplicates automatically.
  • drt_node_pool wrapping the vector<unique_ptr<drt_node>> and its instance is shared across all instances of drt_node to manage the lifetime of new drt_node instances.
  • dynamic_routing_table is to hold one shared_ptr<dynamic_routing_table_data> to let one shared_ptr manage one big object other than multiple shared_ptrs manage multple small objects.

This PR is posted as draft to let @matt-42 have a review first. Will squash to one single commit before final merge.

@cong1920 cong1920 marked this pull request as ready for review December 31, 2024 08:06
@cong1920 cong1920 marked this pull request as draft December 31, 2024 08:07
@cong1920 cong1920 force-pushed the simplify_drt_node branch from 1966842 to 7a5aa97 Compare March 16, 2025 07:58
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant