Skip to content

Commit

Permalink
misc: Reduce allocations and streamline copies
Browse files Browse the repository at this point in the history 
Add AmlSink::vec specialization for Vec so we can memcpy

Convert calls like: `for byte in bytes { sink.byte(byte); }` into
`sink.vec(bytes)`

Remove all calls to Vec::insert by reordering appends or doing a single
memmove and slice copy

Evaluation done by timing the tests and tracking allocations with
valgrind's dhat, both for heap tracking and memcopy tracking.

after/before for total blocks allocated is 6588/8029 = 82%
after/before fastest run time is 4.6/5.9 = 78%

```
set -e
for x in main allocation_reduction; do
    git checkout $x
    echo "=== $x ==="
    cargo test --release --tests --no-run &>/dev/null
    exe=$(find target/release/deps -executable -name 'acpi_tables*')
    sha256sum ${exe}
    taskset -c 0 hyperfine --shell=none "${exe} --test-threads 1"

    valgrind --tool=dhat --mode=heap --dhat-out-file=/dev/null ${exe} \
      --test-threads 1 > /dev/null
    valgrind --tool=dhat --mode=copy --dhat-out-file=/dev/null ${exe} \
      --test-threads 1 > /dev/null
done
```

And the output (with manual removal of extraneous output)

```
=== main ===
Benchmark 1: target/release/deps/acpi_tables-44d55ae4eaed1824
--test-threads 1
  Time (mean ± σ):    6.2 ms ±   0.1 ms    [User: 2.0 ms, System: 2.9
ms]
  Range (min … max):  5.9 ms …   6.7 ms    474 runs

DHAT --mode=heap

Total:     2,452,242 bytes in 8,029 blocks
At t-gmax: 75,063 bytes in 654 blocks
At t-end:  0 bytes in 0 blocks
Reads:     2,953,771 bytes
Writes:    2,194,154 bytes

DHAT --mode=copy

Total:     488,009 bytes in 4,448 blocks

=== allocation_reduction ===
Benchmark 1: target/release/deps/acpi_tables-44d55ae4eaed1824
--test-threads 1
  Time (mean ± σ):    5.0 ms ±   0.2 ms    [User: 1.0 ms, System: 2.8
ms]
  Range (min … max):  4.6 ms …   5.5 ms    587 runs

DHAT --mode=heap

Total:     2,410,141 bytes in 6,588 blocks
At t-gmax: 75,575 bytes in 654 blocks
At t-end:  0 bytes in 0 blocks
Reads:     2,920,895 bytes
Writes:    2,160,911 bytes

DHAT --mode=copy

Total:     1,149,621 bytes in 25,894 blocks
```

Signed-off-by: Andrew Consroe <aconz2@gmail.com>
  • Loading branch information
@aconz2
aconz2 committed Aug 26, 2024
1 parent fe2be0e commit 5dc9449
Show file tree
Hide file tree
Showing 15 changed files with 75 additions and 155 deletions.
133 changes: 47 additions & 86 deletions src/aml.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -141,9 +141,7 @@ impl Aml for Path {
};

for part in self.name_parts.clone().iter_mut() {
for byte in part {
sink.byte(*byte);
}
sink.vec(part);
}
}
}
Expand Down Expand Up @@ -233,9 +231,7 @@ pub struct Name {

impl Aml for Name {
fn to_aml_bytes(&self, sink: &mut dyn AmlSink) {
for byte in &self.bytes {
sink.byte(*byte);
}
sink.vec(&self.bytes);
}
}

Expand Down Expand Up @@ -273,13 +269,10 @@ impl<'a> Aml for Package<'a> {
child.to_aml_bytes(&mut bytes);
}

let mut pkg_length = create_pkg_length(bytes.len(), true);
pkg_length.reverse();
for byte in pkg_length {
bytes.insert(0, byte);
}
let pkg_length = create_pkg_length(bytes.len(), true);

sink.byte(PACKAGEOP);
sink.vec(&pkg_length);
sink.vec(&bytes);
}
}
Expand Down Expand Up @@ -312,6 +305,10 @@ impl AmlSink for PackageBuilder {
fn byte(&mut self, byte: u8) {
self.data.push(byte);
}

fn vec(&mut self, v: &[u8]) {
self.data.extend_from_slice(v);
}
}

impl PackageBuilder {
Expand Down Expand Up @@ -345,13 +342,10 @@ impl<'a> Aml for VarPackageTerm<'a> {
let mut bytes = Vec::new();
self.data.to_aml_bytes(&mut bytes);

let mut pkg_length = create_pkg_length(bytes.len(), true);
pkg_length.reverse();
for byte in pkg_length {
bytes.insert(0, byte);
}
let pkg_length = create_pkg_length(bytes.len(), true);

sink.byte(VARPACKAGEOP);
sink.vec(&pkg_length);
sink.vec(&bytes);
}
}
Expand Down Expand Up @@ -380,7 +374,7 @@ package length is 2**28."

/* Also used for NamedField but in that case the length is not included in itself */
fn create_pkg_length(len: usize, include_self: bool) -> Vec<u8> {
let mut result = Vec::new();
let mut result = Vec::with_capacity(4);

/* PkgLength is inclusive and includes the length bytes */
let length_length = if len < (2usize.pow(6) - 1) {
Expand Down Expand Up @@ -466,9 +460,7 @@ impl Aml for Usize {

fn create_aml_string(v: &str, sink: &mut dyn AmlSink) {
sink.byte(STRINGOP);
for byte in v.as_bytes() {
sink.byte(*byte);
}
sink.vec(v.as_bytes());
sink.byte(0x0); /* NullChar */
}

Expand Down Expand Up @@ -510,21 +502,15 @@ impl<'a> Aml for ResourceTemplate<'a> {

// Buffer length is an encoded integer including buffer data
// and EndTag and checksum byte
let mut buffer_length = Vec::new();
let mut buffer_length = Vec::with_capacity(4);
bytes.len().to_aml_bytes(&mut buffer_length);
buffer_length.reverse();
for byte in buffer_length {
bytes.insert(0, byte);
}

// PkgLength is everything else
let mut pkg_length = create_pkg_length(bytes.len(), true);
pkg_length.reverse();
for byte in pkg_length {
bytes.insert(0, byte);
}
let pkg_length = create_pkg_length(bytes.len() + buffer_length.len(), true);

sink.byte(BUFFEROP);
sink.vec(&pkg_length);
sink.vec(&buffer_length);
sink.vec(&bytes);
}
}
Expand Down Expand Up @@ -806,14 +792,11 @@ impl<'a> Aml for Device<'a> {
child.to_aml_bytes(&mut bytes);
}

let mut pkg_length = create_pkg_length(bytes.len(), true);
pkg_length.reverse();
for byte in pkg_length {
bytes.insert(0, byte);
}
let pkg_length = create_pkg_length(bytes.len(), true);

sink.byte(EXTOPPREFIX); /* ExtOpPrefix */
sink.byte(DEVICEOP); /* DeviceOp */
sink.vec(&pkg_length);
sink.vec(&bytes);
}
}
Expand All @@ -839,13 +822,10 @@ impl<'a> Aml for Scope<'a> {
child.to_aml_bytes(&mut bytes);
}

let mut pkg_length = create_pkg_length(bytes.len(), true);
pkg_length.reverse();
for byte in pkg_length {
bytes.insert(0, byte);
}
let pkg_length = create_pkg_length(bytes.len(), true);

sink.byte(SCOPEOP);
sink.vec(&pkg_length);
sink.vec(&bytes);
}
}
Expand All @@ -859,14 +839,19 @@ impl<'a> Scope<'a> {
/// Create raw bytes representing a Scope from its children in raw bytes
pub fn raw(path: Path, mut children: Vec<u8>) -> Vec<u8> {
let mut bytes = Vec::new();
bytes.push(SCOPEOP);
path.to_aml_bytes(&mut bytes);
bytes.append(&mut children);
let mut pkg_length = create_pkg_length(bytes.len(), true);
pkg_length.reverse();
for byte in pkg_length {
bytes.insert(0, byte);
}
bytes.insert(0, SCOPEOP);

let n = bytes.len(); // n >= 1
let pkg_length = create_pkg_length(n - 1, true);
let m = pkg_length.len();

// move everything after the SCOPEOP over and copy in pkg_length
bytes.resize(n + m, 0xFF);
bytes.as_mut_slice().copy_within(1..n, m + 1);
bytes.as_mut_slice()[1..m + 1].copy_from_slice(pkg_length.as_slice());

bytes
}
}
Expand Down Expand Up @@ -901,13 +886,10 @@ impl<'a> Aml for Method<'a> {
child.to_aml_bytes(&mut bytes);
}

let mut pkg_length = create_pkg_length(bytes.len(), true);
pkg_length.reverse();
for byte in pkg_length {
bytes.insert(0, byte);
}
let pkg_length = create_pkg_length(bytes.len(), true);

sink.byte(METHODOP);
sink.vec(&pkg_length);
sink.vec(&bytes);
}
}
Expand Down Expand Up @@ -996,14 +978,11 @@ impl Aml for Field {
}
}

let mut pkg_length = create_pkg_length(bytes.len(), true);
pkg_length.reverse();
for byte in pkg_length {
bytes.insert(0, byte);
}
let pkg_length = create_pkg_length(bytes.len(), true);

sink.byte(EXTOPPREFIX);
sink.byte(FIELDOP);
sink.vec(&pkg_length);
sink.vec(&bytes);
}
}
Expand Down Expand Up @@ -1078,13 +1057,10 @@ impl<'a> Aml for If<'a> {
child.to_aml_bytes(&mut bytes);
}

let mut pkg_length = create_pkg_length(bytes.len(), true);
pkg_length.reverse();
for byte in pkg_length {
bytes.insert(0, byte);
}
let pkg_length = create_pkg_length(bytes.len(), true);

sink.byte(IFOP);
sink.vec(&pkg_length);
sink.vec(&bytes);
}
}
Expand All @@ -1108,13 +1084,10 @@ impl<'a> Aml for Else<'a> {
child.to_aml_bytes(&mut bytes);
}

let mut pkg_length = create_pkg_length(bytes.len(), true);
pkg_length.reverse();
for byte in pkg_length {
bytes.insert(0, byte);
}
let pkg_length = create_pkg_length(bytes.len(), true);

sink.byte(ELSEOP);
sink.vec(&pkg_length);
sink.vec(&bytes);
}
}
Expand Down Expand Up @@ -1310,13 +1283,10 @@ impl<'a> Aml for While<'a> {
child.to_aml_bytes(&mut bytes);
}

let mut pkg_length = create_pkg_length(bytes.len(), true);
pkg_length.reverse();
for byte in pkg_length {
bytes.insert(0, byte);
}
let pkg_length = create_pkg_length(bytes.len(), true);

sink.byte(WHILEOP);
sink.vec(&pkg_length);
sink.vec(&bytes);
}
}
Expand Down Expand Up @@ -1530,13 +1500,10 @@ impl<'a> Aml for BufferTerm<'a> {
let mut bytes = Vec::new();
self.data.to_aml_bytes(&mut bytes);

let mut pkg_length = create_pkg_length(bytes.len(), true);
pkg_length.reverse();
for byte in pkg_length {
bytes.insert(0, byte);
}
let pkg_length = create_pkg_length(bytes.len(), true);

sink.byte(BUFFEROP);
sink.vec(&pkg_length);
sink.vec(&bytes);
}
}
Expand All @@ -1559,13 +1526,10 @@ impl Aml for BufferData {
self.data.len().to_aml_bytes(&mut bytes);
bytes.extend_from_slice(&self.data);

let mut pkg_length = create_pkg_length(bytes.len(), true);
pkg_length.reverse();
for byte in pkg_length {
bytes.insert(0, byte);
}
let pkg_length = create_pkg_length(bytes.len(), true);

sink.byte(BUFFEROP);
sink.vec(&pkg_length);
sink.vec(&bytes);
}
}
Expand Down Expand Up @@ -1683,14 +1647,11 @@ impl<'a> Aml for PowerResource<'a> {
}

// PkgLength
let mut pkg_length = create_pkg_length(bytes.len(), true);
pkg_length.reverse();
for byte in pkg_length {
bytes.insert(0, byte);
}
let pkg_length = create_pkg_length(bytes.len(), true);

sink.byte(POWERRESOURCEOP);
sink.byte(EXTOPPREFIX);
sink.vec(&pkg_length);
sink.vec(&bytes);
}
}
Expand Down
4 changes: 1 addition & 3 deletions src/cedt.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -302,9 +302,7 @@ impl Aml for CxlFixedMemory {
sink.word(self.window_restrictions);
sink.word(self.qtg_id);
for target in &self.interleave_targets {
for byte in target {
sink.byte(*byte);
}
sink.vec(target);
}
}
}
Expand Down
4 changes: 1 addition & 3 deletions src/fadt.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -147,9 +147,7 @@ impl FADT {

impl Aml for FADT {
fn to_aml_bytes(&self, sink: &mut dyn AmlSink) {
for byte in self.table.as_bytes() {
sink.byte(*byte);
}
sink.vec(self.table.as_bytes());
}
}

Expand Down
20 changes: 8 additions & 12 deletions src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,21 +49,15 @@ pub trait AmlSink {
fn byte(&mut self, byte: u8);

fn word(&mut self, word: u16) {
for byte in word.to_le_bytes() {
self.byte(byte);
}
self.vec(&word.to_le_bytes());
}

fn dword(&mut self, dword: u32) {
for byte in dword.to_le_bytes() {
self.byte(byte);
}
self.vec(&dword.to_le_bytes());
}

fn qword(&mut self, qword: u64) {
for byte in qword.to_le_bytes() {
self.byte(byte);
}
self.vec(&qword.to_le_bytes());
}

fn vec(&mut self, v: &[u8]) {
Expand All @@ -87,6 +81,10 @@ impl AmlSink for alloc::vec::Vec<u8> {
fn byte(&mut self, byte: u8) {
self.push(byte);
}

fn vec(&mut self, v: &[u8]) {
self.extend_from_slice(v);
}
}

/// Standard header for many ACPI tables
Expand Down Expand Up @@ -187,9 +185,7 @@ macro_rules! aml_as_bytes {
($x:ty) => {
impl Aml for $x {
fn to_aml_bytes(&self, sink: &mut dyn AmlSink) {
for byte in self.as_bytes() {
sink.byte(*byte);
}
sink.vec(self.as_bytes());
}
}
};
Expand Down
4 changes: 1 addition & 3 deletions src/madt.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -126,9 +126,7 @@ impl MADT {

impl Aml for MADT {
fn to_aml_bytes(&self, sink: &mut dyn AmlSink) {
for byte in self.header.as_bytes() {
sink.byte(*byte);
}
sink.vec(self.header.as_bytes());

for st in &self.structures {
st.to_aml_bytes(sink);
Expand Down
4 changes: 1 addition & 3 deletions src/mcfg.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -73,9 +73,7 @@ impl MCFG {

impl Aml for MCFG {
fn to_aml_bytes(&self, sink: &mut dyn AmlSink) {
for byte in self.header.as_bytes() {
sink.byte(*byte);
}
sink.vec(self.header.as_bytes());

// 8 reserved bytes
sink.qword(0);
Expand Down
Loading

0 comments on commit 5dc9449

Please sign in to comment.