feat(perf): features for evaluation by nokosaaan · Pull Request #678 · tier4/awkernel

nokosaaan · 2026-04-15T10:09:16Z

Description

Since period information is required during evaluation, I have provided a feature to enable evaluation outside of the application.
period-index-propagation is normally turned off, which is consistent with the behavior of the traditional awkernel. To enable it, simply add period-index-propagation to the default in the [features] section of kernel/Cargo.toml.

I have added perf as a feature for rv32 and rv64, but this is a temporary measure to pass tests on GitHub and is not intended to permanently add perf functionality to these targets.
The root cause of this issue has not yet been resolved and will be tracked in a separate issue.

I have updated pubsub.rs to allow cycle propagation without altering existing trait boundaries, APIs, or the test application. Please refer to the file attached to the PR test section for the execution results.

How was this PR tested?

no_feature_compile.log
no_feature_execute.log
feature_compile.log
feature_execute.log

Notes for reviewers

Signed-off-by: nokosaaan <nishimura.r.019@ms.saitama-u.ac.jp>

…ture

Signed-off-by: nokosaaan <nishimura.r.019@ms.saitama-u.ac.jp>

…ta with features Signed-off-by: nokosaaan <nishimura.r.019@ms.saitama-u.ac.jp>

the logic for passing periodic information has been internalized and solve bootloader error changing const numbers by using Box Signed-off-by: nokosaaan <nishimura.r.019@ms.saitama-u.ac.jp>

Signed-off-by: nokosaaan <nishimura.r.019@ms.saitama-u.ac.jp>

…ture

Signed-off-by: nokosaaan <nishimura.r.019@ms.saitama-u.ac.jp>

kobayu858 · 2026-05-20T13:09:25Z

+        }
+    }
+
+    fn update_time_and_state_for_dag(next_state: PerfState) {


update_time_and_state_for_dag is an exact duplicate of update_time_and_state — not a single line differs. Despite the name implying DAG-specific behavior, both paths in start_interrupt and start_context_switch (branched via dag_info.is_some()) ultimately reach the same logic.

Please remove update_time_and_state_for_dag and consolidate both paths to call update_time_and_state directly.

If we remove update_time_and_state_for_dag and merge it into update_time_and_state, do we no longer need to worry about the possibility that the code in start_interrupt, start_context_switch, and start_context_switch_main might be perceived as containing redundant if-else branches?
Or is the intent of this comment to eliminate all if-else branches and simply change the code to call update_time_and_state?

For now, I’ve removed the if-else handling and made it so that update_time_and_state is called regardless of which task is received. Please let me know if this change was unintended.

Yes, that's exactly what was intended. Since update_time_and_state_for_dag and update_time_and_state were identical, the if-else branches in start_interrupt, start_context_switch, and start_context_switch_main had no practical effect. Removing the branches and calling update_time_and_state directly is the correct fix. Thank you for making the change.

If you have a specific reason for wanting to keep update_time_and_state_for_dag, please let me know. In that case, please revert the change.

I see. If calling the same function—whether in a regular task or a DAG task—doesn’t affect previous evaluation results, then I’m in favor of this change as well.

kobayu858 · 2026-05-20T13:15:52Z

+    const MAX_NODES: usize = 20;
+    #[derive(Clone)]
+    struct PubSubTable {
+        timestamps: [[[u64; MAX_NODES]; MAX_PUBSUB]; MAX_LOGS],


Please add comments explaining the rationale behind MAX_LOGS, MAX_PUBSUB, and MAX_NODES, and whether there is any room for tuning them. As it stands, these constants result in a static allocation of roughly 3.75 MB (8192 × 3 × 20 × 8 bytes), which is fairly large and warrants some justification.

MAX_LOGS and MAX_NODES are adjustable constants, and I have left a comment to that effect. (Since MAX_LOGS is usually fine even if it is a bit smaller, I changed the default to 2048. The value of 2048 was chosen based on this branch.)

The value of MAX_NODES was changed to 5.
This is because the test_dag test application, which runs the DAG, has 5 nodes. (If the user has configured a DAG with 10 nodes, this value can be changed to 10, or if the maximum number of nodes is updated frequently, it can be raised to a value that poses no issues.)

Please add a comment explaining the rationale behind MAX_LOGS = 2048 and clarifying that this is a ring buffer.

I added a comment.

kobayu858 · 2026-05-20T13:18:46Z

@@ -92,12 +93,14 @@ x86 = [
    "awkernel_drivers/uart_16550",
 ]
 rv32 = [
+    "perf",
    "no_std_unwinding",
    "awkernel_lib/rv32",
    "dep:ns16550a",
    "awkernel_drivers/rv32",
 ]
 rv64 = [
+    "perf",
    "no_std_unwinding",
    "awkernel_lib/rv64",
    "dep:ns16550a",


Please address the following:
① Update the PR description — State explicitly that perf was added as a temporary workaround because tests do not pass on rv32/rv64 without it, and that the root cause has not yet been investigated. Leaving the intent undocumented will confuse anyone who looks at this later.
② Add a TODO comment

rv32 = [ # TODO: Remove once a way to pass tests without `perf` is found "perf", ... ]

③ Open a separate issue — It is recommended to track the investigation of why tests fail without perf on RISC-V in a dedicated issue. Left as-is, RISC-V builds will permanently carry the perf feature, which is likely unintended.

…ture Co-authored-by: Copilot <copilot@github.com>

Signed-off-by: nokosaaan <nishimura.r.019@ms.saitama-u.ac.jp>

…ontext_switch etc.. Signed-off-by: nokosaaan <nishimura.r.019@ms.saitama-u.ac.jp>

Signed-off-by: nokosaaan <nishimura.r.019@ms.saitama-u.ac.jp>

kobayu858 · 2026-05-21T14:00:13Z

+        let recorder =
+            recorder_opt.get_or_insert_with(|| Box::new(core::array::from_fn(|_| BTreeMap::new())));
+
+        if recorder[log_index].get(&dag_id).copied().unwrap_or(0) == 0 {


Timestamp 0 is used as a sentinel value meaning "not recorded", but this cannot be distinguished from a valid value if the system uptime happens to be near 0 ns at startup (unlikely in practice, but theoretically possible). Please either consider an alternative approach or add a comment acknowledging this limitation.

I fixed it.

kobayu858 · 2026-05-21T14:03:50Z

+                        let item = $idx.recv().await;
+                        match period_index {
+                            Some(expected) => {
+                                if expected != item.period_index {


The mismatch handling here (adopt first, warn for rest) looks fine, but could we add a comment explaining the intent?

I added a comment.

kobayu858 · 2026-05-21T14:04:40Z

+
        loop {
+            #[cfg(feature = "period-index-propagation")]
+            if last_print.elapsed().as_secs() >= 30 {


This is a magic number. Please either introduce a named constant such as const PERF_PRINT_INTERVAL_SECS: u64 = 30; or make it configurable.

I fixed it.

kobayu858 · 2026-05-21T14:06:10Z

+    const MAX_NODES: usize = 20;
+    #[derive(Clone)]
+    struct PubSubTable {
+        timestamps: [[[u64; MAX_NODES]; MAX_PUBSUB]; MAX_LOGS],


Please add a comment explaining the rationale behind MAX_LOGS = 2048 and clarifying that this is a ring buffer.

Signed-off-by: nokosaaan <nishimura.r.019@ms.saitama-u.ac.jp>

nokosaaan added 3 commits April 15, 2026 18:20

fix: add need-get-period feature

3aca1f9

Signed-off-by: nokosaaan <nishimura.r.019@ms.saitama-u.ac.jp>

fix: delete the portion of impl about macro

87742e5

Signed-off-by: nokosaaan <nishimura.r.019@ms.saitama-u.ac.jp>

fix: delete unnecessary imports

5d30fc2

Signed-off-by: nokosaaan <nishimura.r.019@ms.saitama-u.ac.jp>

github-actions Bot assigned nokosaaan Apr 15, 2026

nokosaaan added 2 commits April 17, 2026 12:58

Merge branch 'main' of https://github.com/tier4/awkernel into eva-fea…

a1f0619

…ture

fix: attach cargo fmt 1

59b5354

Signed-off-by: nokosaaan <nishimura.r.019@ms.saitama-u.ac.jp>

nokosaaan changed the title ~~add: Features for evaluation~~ add: features for evaluation Apr 17, 2026

nokosaaan added 5 commits April 17, 2026 16:37

fix: delete if-else diversion and focus feature diversion

b77385e

Signed-off-by: nokosaaan <nishimura.r.019@ms.saitama-u.ac.jp>

fix: apply cargo fmt 2

85d9042

Signed-off-by: nokosaaan <nishimura.r.019@ms.saitama-u.ac.jp>

fix: experimental apply for perf

c88633a

Signed-off-by: nokosaaan <nishimura.r.019@ms.saitama-u.ac.jp>

fix: experimental apply for perf 2

8495759

Signed-off-by: nokosaaan <nishimura.r.019@ms.saitama-u.ac.jp>

fix: delete .clone() which has Copy trait

8e03ef4

Signed-off-by: nokosaaan <nishimura.r.019@ms.saitama-u.ac.jp>

nokosaaan changed the title ~~add: features for evaluation~~ fix(perf): features for evaluation Apr 17, 2026

nokosaaan added 11 commits April 17, 2026 17:16

fix: unused imports and unnecessary casting

a728ff8

Signed-off-by: nokosaaan <nishimura.r.019@ms.saitama-u.ac.jp>

fix: apply cargo fmt 3

6585af5

Signed-off-by: nokosaaan <nishimura.r.019@ms.saitama-u.ac.jp>

fix: delete unnecessary import

bc19c69

Signed-off-by: nokosaaan <nishimura.r.019@ms.saitama-u.ac.jp>

fix: delete unnecessary functions and modify kernel/src/main.rs

adfa4fd

Signed-off-by: nokosaaan <nishimura.r.019@ms.saitama-u.ac.jp>

fix: add get_period_count for getting periodic period

377d1b1

Signed-off-by: nokosaaan <nishimura.r.019@ms.saitama-u.ac.jp>

fix: async_lib/Cargo.toml

ddd69c2

Signed-off-by: nokosaaan <nishimura.r.019@ms.saitama-u.ac.jp>

fix: apply cargo fmt 4

37cae77

Signed-off-by: nokosaaan <nishimura.r.019@ms.saitama-u.ac.jp>

fix: delete NodeRecord

3ff0804

Signed-off-by: nokosaaan <nishimura.r.019@ms.saitama-u.ac.jp>

fix: unify start publish_timestamp_at to pubsub.rs

d8e28cd

Signed-off-by: nokosaaan <nishimura.r.019@ms.saitama-u.ac.jp>

fix: delete unused imports

2de13a9

Signed-off-by: nokosaaan <nishimura.r.019@ms.saitama-u.ac.jp>

fix: add dependics in kernel/Cargo.toml for print_timestamp_table

9a9d2da

Signed-off-by: nokosaaan <nishimura.r.019@ms.saitama-u.ac.jp>

nokosaaan requested a review from Copilot April 17, 2026 11:02

Copilot started reviewing on behalf of nokosaaan April 17, 2026 11:03 View session

This comment was marked as outdated.

Sign in to view

nokosaaan added 3 commits April 17, 2026 21:31

fix: clarify the responsibility between send_all and send_all_with_me…

14f92ca

…ta with features Signed-off-by: nokosaaan <nishimura.r.019@ms.saitama-u.ac.jp>

fix: delete get_period and

968c058

the logic for passing periodic information has been internalized and solve bootloader error changing const numbers by using Box Signed-off-by: nokosaaan <nishimura.r.019@ms.saitama-u.ac.jp>

fix: apply cargo fmt 5

efc54b6

Signed-off-by: nokosaaan <nishimura.r.019@ms.saitama-u.ac.jp>

nokosaaan added 6 commits May 20, 2026 11:12

fix: renaming variables 1

8ff460b

Signed-off-by: nokosaaan <nishimura.r.019@ms.saitama-u.ac.jp>

Merge branch 'main' of https://github.com/tier4/awkernel into eva-fea…

96c8a11

…ture

fix: apply cargo fmt 8

aae485c

Signed-off-by: nokosaaan <nishimura.r.019@ms.saitama-u.ac.jp>

fix: renaming variables and feature name 2

c657097

Signed-off-by: nokosaaan <nishimura.r.019@ms.saitama-u.ac.jp>

fix: renaming valiables 3 and apply cargo fmt 9

d4a2814

Signed-off-by: nokosaaan <nishimura.r.019@ms.saitama-u.ac.jp>

fix: renaming valiables 4

40270fa

Signed-off-by: nokosaaan <nishimura.r.019@ms.saitama-u.ac.jp>

kobayu858 reviewed May 20, 2026

View reviewed changes

Comment thread awkernel_async_lib/src/dag.rs Outdated

Comment thread awkernel_async_lib/src/dag.rs Outdated

nokosaaan added 2 commits May 20, 2026 13:34

fix: renaming valiables 5

96c3263

Signed-off-by: nokosaaan <nishimura.r.019@ms.saitama-u.ac.jp>

fix: renaming valiables 6 for period-index-propagation

f8b4d38

Signed-off-by: nokosaaan <nishimura.r.019@ms.saitama-u.ac.jp>

kobayu858 changed the title ~~fix(perf): features for evaluation~~ feat(perf): features for evaluation May 20, 2026

kobayu858 requested changes May 20, 2026

View reviewed changes

nokosaaan and others added 12 commits May 20, 2026 22:36

Merge branch 'main' of https://github.com/tier4/awkernel into eva-fea…

78839af

…ture Co-authored-by: Copilot <copilot@github.com>

fix: apply cargo fmt 9

b4a7cbf

Signed-off-by: nokosaaan <nishimura.r.019@ms.saitama-u.ac.jp>

fix: reimpl deleted contextswitchmain by merge conflict

b09da48

Signed-off-by: nokosaaan <nishimura.r.019@ms.saitama-u.ac.jp>

fix: time_interval

39ba2bb

Signed-off-by: nokosaaan <nishimura.r.019@ms.saitama-u.ac.jp>

fix: add comment for test code

4da1730

Signed-off-by: nokosaaan <nishimura.r.019@ms.saitama-u.ac.jp>

fix: assert to log::warn!

19874cd

Signed-off-by: nokosaaan <nishimura.r.019@ms.saitama-u.ac.jp>

fix: delete if-else handling with dag_info in start_interrupt,start_c…

2824a24

…ontext_switch etc.. Signed-off-by: nokosaaan <nishimura.r.019@ms.saitama-u.ac.jp>

fix: update_time_and_state_for_dag

4160343

Signed-off-by: nokosaaan <nishimura.r.019@ms.saitama-u.ac.jp>

fix: delete unused import

4408799

Signed-off-by: nokosaaan <nishimura.r.019@ms.saitama-u.ac.jp>

fix: add comment on MAX_LOGS, MAX_PUBSUB, MAX_NODES

bbc863e

Signed-off-by: nokosaaan <nishimura.r.019@ms.saitama-u.ac.jp>

fix: change value of MAX_NODES

b89e91d

Signed-off-by: nokosaaan <nishimura.r.019@ms.saitama-u.ac.jp>

fix: assert to log::warn!

2845e6a

Signed-off-by: nokosaaan <nishimura.r.019@ms.saitama-u.ac.jp>

kobayu858 requested changes May 21, 2026

View reviewed changes

nokosaaan added 6 commits May 22, 2026 11:55

fix: add comment on MAX_LOGS

c5ea68f

Signed-off-by: nokosaaan <nishimura.r.019@ms.saitama-u.ac.jp>

fix: change logic of logging recorder

0945616

Signed-off-by: nokosaaan <nishimura.r.019@ms.saitama-u.ac.jp>

fix: add comment to recv_all_with_period_index

3277d25

Signed-off-by: nokosaaan <nishimura.r.019@ms.saitama-u.ac.jp>

fix: add magic number to valiable

27699d7

Signed-off-by: nokosaaan <nishimura.r.019@ms.saitama-u.ac.jp>

fix: logging way following ci build test

6ab7887

Signed-off-by: nokosaaan <nishimura.r.019@ms.saitama-u.ac.jp>

fix: add issue link

4e0ff80

Signed-off-by: nokosaaan <nishimura.r.019@ms.saitama-u.ac.jp>

Conversation

nokosaaan commented Apr 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Related links

How was this PR tested?

Notes for reviewers

Uh oh!

This comment was marked as outdated.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nokosaaan May 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nokosaaan May 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

nokosaaan commented Apr 15, 2026 •

edited

Loading

nokosaaan May 20, 2026 •

edited

Loading

nokosaaan May 20, 2026 •

edited

Loading