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

Advance resource management #320

Merged
merged 19 commits into from
Nov 20, 2024
Merged

Conversation

qzhuyan
Copy link
Collaborator

@qzhuyan qzhuyan commented Nov 15, 2024

Tend to decopule the lifetime of msquic handle and nif resource context.
The resource in msquic could be released earlier before the deallocation of erlang NIF resource with some cost of extra refcnts overheads.

Benefits:

  1. For correctness, do not rely on the internal ref countings in MsQuic, take it as black box.
  2. Further reduce memory footprints at server side, adapt to more dynamic environments.
  3. simplify the logic of resource management at both msquic side and erlang nif side.
  4. make it more flexible to operate the stack, support more dynamic configuration.

TODOs:

  • list Breaking changes:
  • update API document
  • update internal dev doc
  • check coverage

targeting ver: 0.2

c_src/quicer_ctx.c Fixed Show fixed Hide fixed
c_src/quicer_listener.c Fixed Show fixed Hide fixed
c_src/quicer_listener.c Fixed Show fixed Hide fixed
c_src/quicer_listener.c Fixed Show fixed Hide fixed
@qzhuyan qzhuyan force-pushed the feat/william/adv-res-mgmt branch from 406d339 to 65a5c98 Compare November 15, 2024 11:06
@qzhuyan qzhuyan force-pushed the feat/william/adv-res-mgmt branch from 65a5c98 to 31031a2 Compare November 15, 2024 11:08
@coveralls
Copy link

coveralls commented Nov 15, 2024

Pull Request Test Coverage Report for Build 11929767256

Details

  • 328 of 371 (88.41%) changed or added relevant lines in 13 files are covered.
  • 18 unchanged lines in 6 files lost coverage.
  • Overall coverage increased (+0.1%) to 86.354%

Changes Missing Coverage Covered Lines Changed/Added Lines %
c_src/quicer_dgram.c 7 8 87.5%
c_src/quicer_stream.c 22 23 95.65%
c_src/quicer_ctx.c 70 73 95.89%
c_src/quicer_nif.c 20 23 86.96%
src/quicer_nif.erl 0 3 0.0%
src/quicer_connection.erl 0 5 0.0%
c_src/quicer_listener.c 41 47 87.23%
c_src/quicer_reg.c 54 63 85.71%
c_src/quicer_connection.c 87 99 87.88%
Files with Coverage Reduction New Missed Lines %
c_src/quicer_reg.c 2 87.05%
c_src/quicer_connection.c 2 86.46%
c_src/quicer_listener.c 3 88.28%
c_src/quicer_config.c 3 87.49%
src/quicer.erl 4 91.08%
c_src/quicer_nif.c 4 80.51%
Totals Coverage Status
Change from base Build 11596714041: 0.1%
Covered Lines: 4012
Relevant Lines: 4646

💛 - Coveralls

c_src/quicer_ctx.h Outdated Show resolved Hide resolved
c_src/quicer_listener.c Outdated Show resolved Hide resolved
c_src/quicer_listener.c Outdated Show resolved Hide resolved
c_src/quicer_listener.c Outdated Show resolved Hide resolved
c_src/quicer_listener.c Show resolved Hide resolved
c_src/quicer_listener.c Outdated Show resolved Hide resolved
c_src/quicer_listener.c Outdated Show resolved Hide resolved
c_src/quicer_listener.c Outdated Show resolved Hide resolved
c_src/quicer_listener.c Outdated Show resolved Hide resolved
c_src/quicer_listener.c Show resolved Hide resolved
@qzhuyan qzhuyan marked this pull request as ready for review November 20, 2024 09:24
@qzhuyan qzhuyan merged commit d1eac4b into emqx:main Nov 20, 2024
44 of 45 checks passed
@qzhuyan qzhuyan deleted the feat/william/adv-res-mgmt branch November 20, 2024 11:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants