[PR #1399] [MERGED] Remove a lot of bounds checks in BinDecoder by tracking position with a second slice #2292

Closed
opened 2026-03-16 08:45:54 +03:00 by kerem · 0 comments
Owner

📋 Pull Request Information

Original PR: https://github.com/hickory-dns/hickory-dns/pull/1399
Author: @saethlin
Created: 3/5/2021
Status: Merged
Merged: 3/11/2021
Merged by: @djc

Base: mainHead: shrink-errors


📝 Commits (2)

  • 65df2d8 Implement BinDecoder with a pair of slices
  • fdf2420 pop using slice::split_first()

📊 Changes

1 file changed (+27 additions, -33 deletions)

View changed files

📝 crates/proto/src/serialize/binary/decoder.rs (+27 -33)

📄 Description

This branch is now awkwardly-named. It used to be a lot of changes, but I'm trickling them in one PR at a time. I don't mind the name, but I'm happy to replace the PR if y'all care.

The representation of BinDecoder incurred an extra bounds check on every operation, which was guaranteed to succeed. Using two slices (one is the original so that it can backtrack) to represent the state removes one bounds check on every access, and this also adds an assert to read_u32 and read_i32 to reduce the number of bounds checks in those.

This change to the representation of BinDecoder produces a ~19% improvement in the message-parsing code. This is not so much because the bounds checks were actually that much overhead; removing the checks shrinks the code size of many BinDecoder methods enough that LLVM decides to inline them where it didn't before.


🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.

## 📋 Pull Request Information **Original PR:** https://github.com/hickory-dns/hickory-dns/pull/1399 **Author:** [@saethlin](https://github.com/saethlin) **Created:** 3/5/2021 **Status:** ✅ Merged **Merged:** 3/11/2021 **Merged by:** [@djc](https://github.com/djc) **Base:** `main` ← **Head:** `shrink-errors` --- ### 📝 Commits (2) - [`65df2d8`](https://github.com/hickory-dns/hickory-dns/commit/65df2d85ca10a04d255efa39dec5d6e824c8e952) Implement BinDecoder with a pair of slices - [`fdf2420`](https://github.com/hickory-dns/hickory-dns/commit/fdf2420f5732cda7946b60444643b96a138ad2db) pop using slice::split_first() ### 📊 Changes **1 file changed** (+27 additions, -33 deletions) <details> <summary>View changed files</summary> 📝 `crates/proto/src/serialize/binary/decoder.rs` (+27 -33) </details> ### 📄 Description This branch is now awkwardly-named. It used to be a lot of changes, but I'm trickling them in one PR at a time. I don't mind the name, but I'm happy to replace the PR if y'all care. The representation of `BinDecoder` incurred an extra bounds check on every operation, which was guaranteed to succeed. Using two slices (one is the original so that it can backtrack) to represent the state removes one bounds check on every access, and this also adds an assert to `read_u32` and `read_i32` to reduce the number of bounds checks in those. This change to the representation of `BinDecoder` produces a ~19% improvement in the message-parsing code. This is not so much because the bounds checks were actually that much overhead; removing the checks shrinks the code size of many `BinDecoder` methods enough that LLVM decides to inline them where it didn't before. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
kerem 2026-03-16 08:45:54 +03:00
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
starred/hickory-dns#2292
No description provided.