Skip to content

Commit 7f9883d

Browse files
committed
Merge branch 'bloat-integration' into hash-owned
2 parents fe168a3 + e95c3da commit 7f9883d

File tree

8 files changed

+2263
-5
lines changed

8 files changed

+2263
-5
lines changed

.github/copilot-instructions.md

Lines changed: 123 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,123 @@
1+
# Copilot Instructions for aspeed-ddk
2+
3+
## Project Overview
4+
aspeed-ddk is a Rust-based driver development kit for ASPEED SoCs, focusing on no_std environments and efficient resource usage.
5+
6+
## Pull Request Review Checklist
7+
8+
- [ ] Code is completely panic-free (no unwrap/expect/panic/indexing)
9+
- [ ] All fallible operations return Result or Option
10+
- [ ] Integer operations use checked/saturating/wrapping methods where needed
11+
- [ ] Array/slice access uses get() or pattern matching, not direct indexing
12+
- [ ] Error cases are well documented and handled appropriately
13+
- [ ] Tests verify error handling paths, not just happy paths
14+
15+
## Quick Reference: Forbidden Patterns
16+
17+
| Forbidden Pattern | Required Alternative |
18+
|-------------------|----------------------|
19+
| `value.unwrap()` | `match value { Some(v) => v, None => return Err(...) }` |
20+
| `result.expect("msg")` | `match result { Ok(v) => v, Err(e) => return Err(e.into()) }` |
21+
| `collection[index]` | `collection.get(index).ok_or(Error::OutOfBounds)?` |
22+
23+
24+
## Code Style
25+
26+
### no_std and Memory Allocation Guidelines
27+
28+
- This project is strictly **no_std** and **no_alloc** in production code
29+
- All production paths must be allocation-free and compatible with bare-metal targets
30+
31+
#### Production Code Requirements
32+
33+
- **NEVER** use crates or features that require heap allocation in production code
34+
- **DO NOT** use the `heapless` crate in production paths despite its name suggesting compatibility
35+
- **DO NOT** use any crate that depends on the `alloc` crate without feature gating
36+
- **ALWAYS** use fixed-size arrays, slices, or static memory allocation
37+
- **ALWAYS** design APIs to accept and return memory provided by the caller
38+
39+
#### Memory Management in Production Code
40+
41+
- Buffers must be pre-allocated by the caller and passed as slices
42+
- Collection types must have fixed, compile-time sizes
43+
- All data structures must have predictable, static memory footprints
44+
- No dynamic memory growth patterns are allowed
45+
46+
#### Test Code Exceptions
47+
48+
- Test code (annotated with `#[cfg(test)]`) may use allocation if needed
49+
- The `heapless` crate and other no_std compatible collections are permitted in tests
50+
- Standard library features may be used in tests when the `std` feature is enabled
51+
- Test helpers can use more ergonomic APIs that wouldn't be appropriate for production
52+
53+
#### Example: Production vs. Test Code
54+
55+
```rust
56+
// Production code - strict no_alloc approach
57+
pub fn process_data(data: &[u8], output: &mut [u8]) -> Result<usize, Error> {
58+
if output.len() < data.len() * 2 {
59+
return Err(Error::BufferTooSmall);
60+
}
61+
62+
// Process data into output buffer
63+
// ...
64+
65+
Ok(processed_bytes)
66+
}
67+
68+
// Test code - can use more ergonomic approaches
69+
#[cfg(test)]
70+
mod tests {
71+
use super::*;
72+
73+
#[test]
74+
fn test_process_data() {
75+
// It's fine to use Vec in tests
76+
let input = vec![1, 2, 3, 4];
77+
let mut output = vec![0; 16];
78+
79+
let result = process_data(&input, &mut output);
80+
assert!(result.is_ok());
81+
// ...
82+
}
83+
84+
#[test]
85+
fn test_with_heapless() {
86+
// Heapless is also fine in tests
87+
use heapless::Vec;
88+
let mut input: Vec<u8, 8> = Vec::new();
89+
input.extend_from_slice(&[1, 2, 3]).unwrap();
90+
91+
let mut output = [0u8; 16];
92+
let result = process_data(&input, &mut output);
93+
assert!(result.is_ok());
94+
// ...
95+
}
96+
}
97+
98+
99+
### Unsafe Code
100+
101+
- Minimize unsafe code; isolate in well-documented functions
102+
- Document all safety preconditions in unsafe functions
103+
- Add safety comments explaining why unsafe is necessary
104+
- Unit test unsafe code thoroughly
105+
106+
## Common Patterns
107+
108+
### Static vs. Dynamic Dispatch
109+
110+
This project strongly prefers static dispatch over dynamic dispatch to optimize for binary size, performance, and no_std compatibility.
111+
112+
#### Static Dispatch Requirements
113+
114+
- Use generic parameters instead of trait objects (`dyn Trait`) whenever possible
115+
- Leverage impl Trait in return positions rather than Box<dyn Trait>
116+
- Monomorphize code at compile time rather than using virtual dispatch at runtime
117+
- Avoid heap allocations associated with typical dyn Trait usage
118+
119+
120+
## Workflows
121+
122+
### Pre-commit
123+
cargo xtask precommit

.github/workflows/build-test.yml

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,9 @@ jobs:
2828
run: |
2929
sudo apt-get update -qy
3030
sudo apt-get install -qy build-essential curl gcc-multilib gcc-riscv64-unknown-elf git
31+
32+
- name: Install cargo-bloat
33+
run: cargo install cargo-bloat
3134

3235
- name: Verify Cargo.lock is up to date
3336
run: |
@@ -41,3 +44,11 @@ jobs:
4144
- name: Run precommit checks (build/format/lint)
4245
run: |
4346
cargo --config "$EXTRA_CARGO_CONFIG" xtask precommit
47+
48+
- name: Upload binary size reports
49+
uses: actions/upload-artifact@v4
50+
if: always()
51+
with:
52+
name: bloat-reports
53+
path: target/bloat-reports/
54+
retention-days: 30
Lines changed: 106 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,106 @@
1+
# Licensed under the Apache-2.0 license
2+
3+
name: Binary Size Analysis
4+
5+
on:
6+
pull_request:
7+
branches:
8+
- main
9+
paths:
10+
- 'src/**'
11+
- 'Cargo.toml'
12+
- 'Cargo.lock'
13+
14+
jobs:
15+
size-analysis:
16+
runs-on: ubuntu-22.04
17+
18+
permissions:
19+
pull-requests: write
20+
contents: read
21+
22+
steps:
23+
- name: Checkout PR
24+
uses: actions/checkout@v4
25+
with:
26+
fetch-depth: 0
27+
28+
- name: Install packages
29+
run: |
30+
sudo apt-get update -qy
31+
sudo apt-get install -qy build-essential curl gcc-multilib gcc-riscv64-unknown-elf git
32+
33+
- name: Install cargo-bloat
34+
run: cargo install cargo-bloat
35+
36+
- name: Build current branch
37+
run: |
38+
cargo build --release --target thumbv7em-none-eabihf
39+
40+
- name: Analyze current branch size
41+
run: |
42+
cargo xtask bloat --release --report --output-dir target/pr-bloat-reports
43+
44+
- name: Checkout main branch
45+
run: |
46+
git checkout main
47+
48+
- name: Build main branch
49+
run: |
50+
cargo build --release --target thumbv7em-none-eabihf
51+
52+
- name: Analyze main branch size
53+
run: |
54+
cargo xtask bloat --release --report --output-dir target/main-bloat-reports
55+
56+
- name: Generate size comparison report
57+
id: size-comparison
58+
run: |
59+
# Create a comprehensive comparison
60+
echo "## 📊 Binary Size Analysis" > size_report.md
61+
echo "" >> size_report.md
62+
63+
# Get binary sizes
64+
PR_SIZE=$(stat -c%s target/thumbv7em-none-eabihf/release/aspeed-ddk 2>/dev/null || echo "0")
65+
git checkout -
66+
MAIN_SIZE=$(stat -c%s target/thumbv7em-none-eabihf/release/aspeed-ddk 2>/dev/null || echo "0")
67+
68+
# Calculate difference
69+
SIZE_DIFF=$((PR_SIZE - MAIN_SIZE))
70+
71+
echo "### Size Comparison" >> size_report.md
72+
echo "- **Main branch**: $(numfmt --to=iec $MAIN_SIZE)" >> size_report.md
73+
echo "- **PR branch**: $(numfmt --to=iec $PR_SIZE)" >> size_report.md
74+
echo "- **Difference**: $(numfmt --to=iec $SIZE_DIFF)" >> size_report.md
75+
echo "" >> size_report.md
76+
77+
if [ $SIZE_DIFF -gt 10240 ]; then
78+
echo "⚠️ **Warning**: Binary size increased by more than 10KB" >> size_report.md
79+
echo "size_warning=true" >> $GITHUB_OUTPUT
80+
elif [ $SIZE_DIFF -gt 0 ]; then
81+
echo "ℹ️ Binary size increased slightly" >> size_report.md
82+
elif [ $SIZE_DIFF -lt 0 ]; then
83+
echo "✅ Binary size decreased!" >> size_report.md
84+
fi
85+
86+
echo "" >> size_report.md
87+
echo "### Top Functions (Current PR)" >> size_report.md
88+
echo '```' >> size_report.md
89+
head -20 target/pr-bloat-reports/bloat_functions.txt >> size_report.md 2>/dev/null || echo "No function data available" >> size_report.md
90+
echo '```' >> size_report.md
91+
92+
- name: Comment PR with size analysis
93+
uses: thollander/actions-comment-pull-request@v2
94+
with:
95+
filePath: size_report.md
96+
comment_tag: size-analysis
97+
98+
- name: Upload detailed reports
99+
uses: actions/upload-artifact@v4
100+
with:
101+
name: size-analysis-detailed
102+
path: |
103+
target/pr-bloat-reports/
104+
target/main-bloat-reports/
105+
size_report.md
106+
retention-days: 7

docs/size-analysis.md

Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,94 @@
1+
# Binary Size Analysis
2+
3+
This project includes automated binary size analysis to help maintain optimal resource usage for embedded targets.
4+
5+
## Quick Start
6+
7+
### Analyze current build
8+
```bash
9+
# Install cargo-bloat if not already installed
10+
cargo install cargo-bloat
11+
12+
# Basic size analysis
13+
cargo xtask bloat --release
14+
15+
# Generate detailed reports
16+
cargo xtask bloat --release --report
17+
```
18+
19+
### Reading the Reports
20+
21+
Reports are generated in `target/bloat-reports/`:
22+
23+
- `bloat_functions.txt` - Largest functions by size
24+
- `bloat_crates.txt` - Size breakdown by crate
25+
- `size_comparison.md` - Comparison with baselines
26+
27+
## Size Targets
28+
29+
### Current Constraints
30+
- **Flash**: Target < 256KB for production builds
31+
- **RAM**: Target < 64KB total usage
32+
- **Critical path**: Boot code should be < 32KB
33+
34+
### Optimization Guidelines
35+
36+
#### 🟢 Good practices:
37+
- Use `#[inline(never)]` for large, rarely-called functions
38+
- Prefer static dispatch over dynamic dispatch
39+
- Use const generics instead of runtime configuration
40+
- Enable LTO and optimize for size in release builds
41+
42+
#### 🔴 Watch out for:
43+
- Large generic monomorphizations
44+
- Unused features being compiled in
45+
- Debug formatting code in release builds
46+
- Accidental std library usage
47+
48+
## Integration with CI/CD
49+
50+
### Automated Analysis
51+
- **Every PR**: Size comparison against main branch
52+
- **Precommit**: Local size analysis in reports
53+
- **Releases**: Historical size tracking
54+
55+
### Size Regression Detection
56+
PRs that increase binary size by more than 10KB will be flagged for review.
57+
58+
## Manual Investigation
59+
60+
### Finding size regressions:
61+
```bash
62+
# Compare with specific commit
63+
git checkout <baseline-commit>
64+
cargo xtask bloat --release --output-dir target/baseline-reports
65+
66+
git checkout <current-branch>
67+
cargo xtask bloat --release --output-dir target/current-reports
68+
69+
# Manual comparison of the reports
70+
```
71+
72+
### Investigating specific functions:
73+
```bash
74+
# Find largest functions
75+
cargo bloat --release --crates
76+
77+
# Find compilation time hogs (often correlates with code size)
78+
cargo bloat --release --time
79+
```
80+
81+
## Size Budgets by Feature
82+
83+
| Component | Target Size | Current | Notes |
84+
|-----------|-------------|---------|--------|
85+
| Core HAL | < 32KB | TBD | Hardware abstraction |
86+
| Crypto (RSA) | < 64KB | TBD | RSA operations |
87+
| Crypto (ECDSA) | < 32KB | TBD | ECDSA operations |
88+
| Hash functions | < 16KB | TBD | SHA family |
89+
| Main application | < 32KB | TBD | Application logic |
90+
| **Total** | **< 176KB** | **TBD** | Leaves 80KB buffer |
91+
92+
## Historical Tracking
93+
94+
Size data is tracked in CI artifacts and can be used to generate long-term size trend reports.

0 commit comments

Comments
 (0)