From aac8390ecb3e44ce5704d54923ac23d022d8f9b7 Mon Sep 17 00:00:00 2001
From: norbiros <norbiros@protonmail.com>
Date: Sat, 12 Oct 2024 11:26:07 +0200
Subject: [PATCH] refactor: Code review fixes

---
 Cargo.toml          |  1 -
 benches/read.rs     | 23 +++++++++++++++--------
 src/macros.rs       | 12 +++---------
 src/nbt/compound.rs | 18 ++++++++++--------
 4 files changed, 28 insertions(+), 26 deletions(-)

diff --git a/Cargo.toml b/Cargo.toml
index 103ffa6..aa612fc 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -23,7 +23,6 @@ cesu8 = "1.1.0"
 derive_more = { version = "1.0.0", features = ["into", "from"] }
 thiserror = "1.0.63"
 serde = { version = "1.0.209", optional = true, features = ["derive"] }
-rustc-hash = "2.0.0"
 
 [dev-dependencies]
 criterion = { version = "0.5.1", features = ["html_reports"] }
diff --git a/benches/read.rs b/benches/read.rs
index 8496d8e..2569012 100644
--- a/benches/read.rs
+++ b/benches/read.rs
@@ -1,12 +1,12 @@
 use bytes::{Bytes, BytesMut};
 use crab_nbt::Nbt;
-use criterion::{black_box, criterion_group, criterion_main, Criterion, Throughput};
+use criterion::{criterion_group, criterion_main, BatchSize, Criterion, Throughput};
 use flate2::read::GzDecoder;
 use std::fs::File;
 use std::io::Read;
 
-fn criterion_benchmark(c: &mut Criterion) {
-    let mut file = File::open("tests/data/complex_player.dat").expect("Failed to open file");
+fn decompress_data(file_path: &str) -> Vec<u8> {
+    let mut file = File::open(file_path).expect("Failed to open file");
     let mut buffer = Vec::new();
     file.read_to_end(&mut buffer).expect("Failed to read file");
     let mut src = &buffer[..];
@@ -17,17 +17,24 @@ fn criterion_benchmark(c: &mut Criterion) {
         input = buffer;
     }
 
+    input
+}
+
+fn criterion_benchmark(c: &mut Criterion) {
+    let input = decompress_data("tests/data/complex_player.dat");
+
     let bytes = Bytes::from_iter(input);
     let bytes_mut = BytesMut::from(bytes);
 
     let mut group = c.benchmark_group("read");
     group.throughput(Throughput::Bytes(bytes_mut.len() as u64));
 
-    group.bench_function("read_bigtest_nbt", |b| {
-        b.iter(|| {
-            let output = Nbt::read(&mut bytes_mut.clone()).expect("Failed to parse NBT");
-            black_box(output)
-        })
+    group.bench_function("read_complex_player_nbt", |b| {
+        b.iter_batched_ref(
+            || bytes_mut.clone(),
+            |bytes| Nbt::read(bytes).expect("Failed to parse NBT"),
+            BatchSize::SmallInput,
+        )
     });
 }
 
diff --git a/src/macros.rs b/src/macros.rs
index 3008c0f..d419c05 100644
--- a/src/macros.rs
+++ b/src/macros.rs
@@ -63,15 +63,9 @@ macro_rules! nbt {
 macro_rules! nbt_inner {
     ({ $($key:tt : $value:tt),* $(,)? }) => {
         $crate::NbtCompound::from_iter({
-            #[allow(unused_mut)]
-            let mut values = ::std::vec::Vec::<(::std::string::String, $crate::NbtTag)>::new();
-            $(
-                let key: ::std::string::String = $key.into();
-                if !values.iter().any(|(k, _)| k == &key) {
-                    values.push((key, nbt_inner!(@value $value)));
-                }
-            )*
-            values
+            let mut tags = ::std::vec::Vec::<(::std::string::String, $crate::NbtTag)>::new();
+            $(tags.push(($key.into(), nbt_inner!(@value $value)));)*
+            tags
         })
     };
     (@value $ident:ident) => { $crate::NbtTag::from($ident) };
diff --git a/src/nbt/compound.rs b/src/nbt/compound.rs
index cba4a4f..e7193e5 100644
--- a/src/nbt/compound.rs
+++ b/src/nbt/compound.rs
@@ -19,7 +19,7 @@ impl NbtCompound {
     }
 
     pub fn deserialize_content(bytes: &mut impl Buf) -> Result<NbtCompound, Error> {
-        let mut child_tags = Vec::new();
+        let mut compound = NbtCompound::new();
 
         while bytes.has_remaining() {
             let tag_id = bytes.get_u8();
@@ -30,15 +30,13 @@ impl NbtCompound {
             let name = get_nbt_string(bytes)?;
 
             if let Ok(tag) = NbtTag::deserialize_data(bytes, tag_id) {
-                if !child_tags.iter().any(|(key, _)| key == &name) {
-                    child_tags.push((name, tag));
-                }
+                compound.put(name, tag);
             } else {
                 break;
             }
         }
 
-        Ok(NbtCompound { child_tags })
+        Ok(compound)
     }
 
     pub fn deserialize_content_from_cursor(
@@ -64,7 +62,9 @@ impl NbtCompound {
     }
 
     pub fn put(&mut self, name: String, value: impl Into<NbtTag>) {
-        self.child_tags.push((name, value.into()));
+        if !self.child_tags.iter().any(|(key, _)| key == &name) {
+            self.child_tags.push((name, value.into()));
+        }
     }
 
     pub fn get_byte(&self, name: &str) -> Option<i8> {
@@ -134,9 +134,11 @@ impl From<Nbt> for NbtCompound {
 
 impl FromIterator<(String, NbtTag)> for NbtCompound {
     fn from_iter<T: IntoIterator<Item = (String, NbtTag)>>(iter: T) -> Self {
-        Self {
-            child_tags: Vec::from_iter(iter),
+        let mut compound = NbtCompound::new();
+        for (key, value) in iter {
+            compound.put(key, value);
         }
+        compound
     }
 }