Fixes #799: Split the pooled type declarations into weak-reffed and the non-weak-reffed #564
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jiridanek
changed the title
jiridanek
requested review from
kgiusti and
ganeshmurthy
and removed request for
kgiusti
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #564 +/- ##
===========================================
- Coverage 77.89% 27.90% -49.99%
===========================================
Files 238 133 -105
Lines 60704 31816 -28888
Branches 5580 5091 -489
===========================================
- Hits 47286 8879 -38407
- Misses 10795 21807 +11012
+ Partials 2623 1130 -1493
Flags with carried forward coverage won't be shown. Click here to find out more.
|
jiridanek
commented
Jul 20, 2022
|
/me trying git-absorb, https://github.com/tummychow/git-absorb rename _WEAK to _SAFE, then run and... |
jiridanek
force-pushed
the
jd_2022_06_10_split_pooled_types_into_weakrefed_and_the_rest
branch
from
b7e77dc to
f0c5a11
Compare
|
... one more command is needed to process the generated fixup! commits |
jiridanek
changed the title
jiridanek
changed the title
jiridanek
force-pushed
the
jd_2022_06_10_split_pooled_types_into_weakrefed_and_the_rest
branch
2 times, most recently
from
58bb119 to
70eeae6
Compare
|
AFAIK we do want to do this,
|
jiridanek
force-pushed
the
jd_2022_06_10_split_pooled_types_into_weakrefed_and_the_rest
branch
from
a3e403d to
3f084bc
Compare
jiridanek
force-pushed
the
jd_2022_06_10_split_pooled_types_into_weakrefed_and_the_rest
branch
2 times, most recently
from
ca277ce to
a7bc783
Compare
see qdr_link_set_context (the context may be qd_link_t, qdr_tcp_connection_t, qdr_http2_stream_data_t or qdr_http1_connection_t) see qd_link_set_incoming_msg which uses qd_alloc_set_safe_ptr directly for qd_message_t
jiridanek
force-pushed
the
jd_2022_06_10_split_pooled_types_into_weakrefed_and_the_rest
branch
from
a7bc783 to
6628d6b
Compare
|
@jiridanek Looks like your new rebase fixed the issue and all tests have passed. Please feel free to merge. |
ganeshmurthy
approved these changes
Jun 8, 2023
jiridanek
deleted the
jd_2022_06_10_split_pooled_types_into_weakrefed_and_the_rest
branch
jiridanek
added a commit
to jiridanek/skupper-router
that referenced
this pull request
Sep 19, 2023
…e-pointed and the non-safe-pointed ones (skupperproject#564) * Split out the types that do need to have safe pointers * Add _SAFE for types that nonobviously need it; see `qdr_link_set_context` (the `context` may be `qd_link_t`, `qdr_tcp_connection_t`, `qdr_http2_stream_data_t` or `qdr_http1_connection_t`) see `qd_link_set_incoming_msg` which uses `qd_alloc_set_safe_ptr` directly for `qd_message_t`
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There are three reasons why use memory pool for a type. First, because it makes allocation/deallocation faster, second because it prevents memory fragmentation, and third it allows holding weak references to the type.
The pool is constrained by having to support these use cases. Namely, a pool that supports weak references may never free any memory to the operating system, because it must be always able to dereference any previously obtained weak pointer.
This PR creates a separate API to declare weak-reffed types, so it is now obvious what types need weak references. There are no changes to functionality.
Note the functions mentioned in the second commit, which bypass the usual weak pointer API and mess with weak pointers under the table. This approach makes it look like the types don't get used with weak pointers, but they actually do get weak-pointed. (This only becomes visible when you (for testing purposes) replace pooled allocation for the non-weak types with malloc/free; the way the PR does things, the underhanded approach can still be used to create weak references even though type was not declared to support it.)