Heap-ify ReadHandler pool on the Server #13066
Merged
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
This amongst other things, converts the custom-managed array of ReadHandlers to a ObjectPool<>. This ensures that on platforms that have heap, that will allocate out of that when space runs out. This removes the 4 subscription limit right now on capable platforms that support heap. It also does a number of other side fixes needed to make the above work. Tests: - Validated using the REPL - Wrote a fairly comprehensive E2E unit test that validates the heap implementation as well as the ability to return back resource exhaustion errors.
mrjerryjohns
changed the title
pullapprove
bot
requested review from
anush-apple,
austinh0,
balducci-apple,
Byungjoo-Lee,
bzbarsky-apple,
carol-apple,
chrisdecenzo,
chshu,
chulspro,
Damian-Nordic,
dhrishi,
electrocucaracha,
erjiaqing,
franck-apple,
gjc13,
harimau-qirex,
hawk248,
holbrookt,
jelderton,
jepenven-silabs,
jmartinez-silabs,
kghost,
kpschoedel,
LuDuda and
mlepage-google
mrjerryjohns
force-pushed
the
read-client-lifetime
branch
from
940f82e
to
e4e1779
Compare
|
PR #13066: Size comparison from e25a501 to 308e2d5 Increases above 0.2%:
Increases (27 builds for cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, p6, qpg, telink)
Decreases (1 build for linux)
Full report (33 builds for cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, p6, qpg, telink)
|
mrjerryjohns
force-pushed
the
read-client-lifetime
branch
from
94a77b9
to
939cada
Compare
|
PR #13066: Size comparison from bb2838f to 939cada Increases above 0.2%:
Increases (13 builds for esp32, linux, nrfconnect, telink)
Decreases (1 build for linux)
Full report (16 builds for esp32, linux, nrfconnect, telink)
|
mrjerryjohns
force-pushed
the
read-client-lifetime
branch
from
939cada
to
c5a18a6
Compare
|
PR #13066: Size comparison from 270081a to c5a18a6 Increases (25 builds for cyw30739, efr32, esp32, k32w, mbed, nrfconnect, p6, qpg, telink)
Full report (31 builds for cyw30739, efr32, esp32, k32w, mbed, nrfconnect, p6, qpg, telink)
|
|
PR #13066: Size comparison from be9582b to cdd5849 Increases above 0.2%:
Increases (30 builds for cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, p6, qpg, telink)
Decreases (1 build for linux)
Full report (33 builds for cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, p6, qpg, telink)
|
two timer callbacks, resulting in it firing after the object's destruction and causing all kinds of memory corruption havoc that took 3 hours to debug.
jmartinez-silabs
approved these changes
Jan 26, 2022
|
PR #13066: Size comparison from be9582b to a35b0f6 Increases above 0.2%:
Increases (2 builds for esp32)
Full report (2 builds for esp32)
|
|
Fast tracking, given > 3 days for review. |
|
PR #13066: Size comparison from 246473c to cb58a0a Increases above 0.2%:
Increases (14 builds for cyw30739, efr32, k32w, linux, p6, qpg, telink)
Decreases (1 build for linux)
Full report (15 builds for cyw30739, efr32, k32w, linux, p6, qpg, telink)
|
mrjerryjohns
force-pushed
the
read-client-lifetime
branch
from
cb58a0a
to
78ce55f
Compare
|
PR #13066: Size comparison from 246473c to e0e565a Increases above 0.2%:
Increases (11 builds for cyw30739, k32w, linux, p6, qpg, telink)
Decreases (1 build for linux)
Full report (12 builds for cyw30739, k32w, linux, p6, qpg, telink)
|
selissia
pushed a commit
to selissia/connectedhomeip
that referenced
this pull request
Jan 28, 2022
This was referenced Feb 1, 2022
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.
This is part 1/N to slowly heap-ify pools in the IM.
This PR specifically heap-ifies the
ReadHandlerpool on the server. To achieve that, it converts the custom-managed array of ReadHandlers to anObjectPool<>. This ensures that on platforms that have heap, that will allocate out ofthat when space runs out.
It also fixes a number of other notable issues:
CloseandAbortfor referring to the graceful/abrupt shutdown of these objects.Aborta private member that is not meant to be utilized by applications, since in all cases today, applications only really emit path-specific errors and never really need to teardown an interaction forcefully at any time on the server.Tests: