-
Notifications
You must be signed in to change notification settings - Fork 0
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
Fix/allocate shm only when needed #325
Merged
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
Signed-off-by: Koichi Imai <koichi.imai.2@tier4.jp>
Signed-off-by: Koichi Imai <koichi.imai.2@tier4.jp>
Signed-off-by: Koichi Imai <koichi.imai.2@tier4.jp>
Signed-off-by: Koichi Imai <koichi.imai.2@tier4.jp>
Signed-off-by: Koichi Imai <koichi.imai.2@tier4.jp>
Signed-off-by: Koichi Imai <koichi.imai.2@tier4.jp>
Signed-off-by: Koichi Imai <koichi.imai.2@tier4.jp>
veqcc
reviewed
Jan 20, 2025
veqcc
reviewed
Jan 20, 2025
atsushi421
reviewed
Jan 20, 2025
atsushi421
reviewed
Jan 20, 2025
Signed-off-by: Koichi Imai <koichi.imai.2@tier4.jp>
sykwer
previously approved these changes
Jan 20, 2025
全てのコメントを反映したらLGTM |
atsushi421
previously approved these changes
Jan 21, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
@Koichi98 |
veqcc
reviewed
Jan 21, 2025
Signed-off-by: Koichi Imai <koichi.imai.2@tier4.jp>
veqcc
previously approved these changes
Jan 21, 2025
|
Signed-off-by: Koichi Imai <koichi.imai.2@tier4.jp>
|
Signed-off-by: Koichi Imai <koichi.imai.2@tier4.jp>
|
atsushi421
approved these changes
Jan 21, 2025
atsushi421
pushed a commit
that referenced
this pull request
Jan 23, 2025
* only allocate shared memory between borrow_loaned_message to publish Signed-off-by: Koichi Imai <koichi.imai.2@tier4.jp> * run_listener works but now it sometimes fails with PUBLISHER_ADD_CMD Signed-off-by: Koichi Imai <koichi.imai.2@tier4.jp> * above commit was correct, thus refactoring Signed-off-by: Koichi Imai <koichi.imai.2@tier4.jp> * add error handling for decrement_publisher_num_borrowed Signed-off-by: Koichi Imai <koichi.imai.2@tier4.jp> * refactoring Signed-off-by: Koichi Imai <koichi.imai.2@tier4.jp> * apply clang-format Signed-off-by: Koichi Imai <koichi.imai.2@tier4.jp> * Add comment Signed-off-by: Koichi Imai <koichi.imai.2@tier4.jp> * rename to borrowed_publisher_num & refactor Signed-off-by: Koichi Imai <koichi.imai.2@tier4.jp> * fix for clang-tidy Signed-off-by: Koichi Imai <koichi.imai.2@tier4.jp> * fix Signed-off-by: Koichi Imai <koichi.imai.2@tier4.jp> * apply cargo fmt Signed-off-by: Koichi Imai <koichi.imai.2@tier4.jp> * apply clippy Signed-off-by: Koichi Imai <koichi.imai.2@tier4.jp> --------- Signed-off-by: Koichi Imai <koichi.imai.2@tier4.jp>
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.
Description
現状のAgnocastでは、agnocastを適用したプロセスのヒープ領域は全て共有メモリ (MAP_SHARED) 上に割り当てられるデザインになっているが、これはメモリ効率の観点で問題点がある:一度ヒープとして確保した共有メモリ物理ページが、今後再利用されなかったとしてもずっと解放されず、カーネルが再利用することができない。
madvise(MADV_REMOVE)を利用して、明示的にユーザレイヤから当該物理ページを破棄するように要求することは可能だが、
ミドルウェア (agnocast heaphook) から見える情報だけで MADV_REVOVE 発行タイミングの決定をするのは難しい。アプリケーションのコード上から、アプリケーションの知識に基づき MADV_REVOVE を発行する必要があるが、アプリケーションのプログラマにこのような低レイヤの操作を強いることはできない。
以上を踏まえ、そもそも共有メモリの使用を最低限に抑えるという方針を取ることとなった。これはborrow_loaned_message()からpublish()までの間で確保されるヒープ領域のみを共有メモリ上におくことで実現している。
本実装によって今後のAgnocast適用時には以下の2点に注意する必要がある。
Related links
TIER IV Internal Doc
How was this PR tested?
上記に加えて、Eveにおけるmap_loader -> pose_initializer, ad_service_state_monitor, ndt_scan_matcherへの適用。
Notes for reviewers
borrow_loaned_messageからpublishするまでのみを共有メモリに配置するようになったことで、agnocast_heaphookにおけるTLSF変数の初期化が初回borrow時になるが、initialize_agnocastが呼ばれていないのでこれではcreate_publisherやcreate_subscriptionにおけるioctlすら呼べない。それゆえ、TLSFの初期化はプロセスのmain関数の実行よりも前に行う必要があり、__libc_start_main時に行うこととした。初回mallocではなく__libc_start_mainとした理由は以下。