* [pve-devel] [PATCH installer v3 1/2] tree-wide: run rustfmt, fix clippy warnings
2024-11-14 14:15 [pve-devel] [PATCH installer v3 0/2] assistant: clean up glob patterns & regexes Christoph Heiss
@ 2024-11-14 14:15 ` Christoph Heiss
2024-11-14 14:15 ` [pve-devel] [PATCH installer v3 2/2] assistant: avoid regex for simple prefix matching Christoph Heiss
2024-11-14 19:53 ` [pve-devel] applied: [PATCH installer v3 0/2] assistant: clean up glob patterns & regexes Thomas Lamprecht
2 siblings, 0 replies; 4+ messages in thread
From: Christoph Heiss @ 2024-11-14 14:15 UTC (permalink / raw)
To: pve-devel
No functional changes.
Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
Changes v1 -> v2:
* no changes
Changes v2 -> v3:
* rebased on latest master, mostly new (mechanical) changes
proxmox-auto-install-assistant/src/main.rs | 4 ++--
proxmox-auto-installer/tests/parse-answer.rs | 2 +-
proxmox-installer-common/src/setup.rs | 5 ++++-
proxmox-tui-installer/src/views/mod.rs | 2 +-
4 files changed, 8 insertions(+), 5 deletions(-)
diff --git a/proxmox-auto-install-assistant/src/main.rs b/proxmox-auto-install-assistant/src/main.rs
index bc7d5d8..da1ebda 100644
--- a/proxmox-auto-install-assistant/src/main.rs
+++ b/proxmox-auto-install-assistant/src/main.rs
@@ -474,7 +474,7 @@ fn get_disks() -> Result<BTreeMap<String, BTreeMap<String, String>>> {
}
}
- let output = match get_udev_properties(&entry.path()) {
+ let output = match get_udev_properties(entry.path()) {
Ok(output) => output,
Err(err) => {
eprint!("{err}");
@@ -522,7 +522,7 @@ fn get_nics() -> Result<BTreeMap<String, BTreeMap<String, String>>> {
for link in links {
let path = format!("/sys/class/net/{link}");
- let output = match get_udev_properties(&PathBuf::from(path)) {
+ let output = match get_udev_properties(PathBuf::from(path)) {
Ok(output) => output,
Err(err) => {
eprint!("{err}");
diff --git a/proxmox-auto-installer/tests/parse-answer.rs b/proxmox-auto-installer/tests/parse-answer.rs
index 0e5d6e7..65f8c1e 100644
--- a/proxmox-auto-installer/tests/parse-answer.rs
+++ b/proxmox-auto-installer/tests/parse-answer.rs
@@ -50,7 +50,7 @@ pub fn setup_test_basic(path: impl AsRef<Path>) -> (SetupInfo, LocaleInfo, Runti
fn test_parse_answers() {
let path = get_test_resource_path().unwrap();
let (setup_info, locales, runtime_info, udev_info) = setup_test_basic(&path);
- let mut tests_path = path.clone();
+ let mut tests_path = path;
tests_path.push("parse_answer");
let test_dir = fs::read_dir(tests_path.clone()).unwrap();
diff --git a/proxmox-installer-common/src/setup.rs b/proxmox-installer-common/src/setup.rs
index cd1e8b4..79b17ed 100644
--- a/proxmox-installer-common/src/setup.rs
+++ b/proxmox-installer-common/src/setup.rs
@@ -12,7 +12,10 @@ use std::{
use serde::{de, Deserialize, Deserializer, Serialize, Serializer};
use crate::{
- options::{BtrfsBootdiskOptions, BtrfsCompressOption, Disk, FsType, ZfsBootdiskOptions, ZfsChecksumOption, ZfsCompressOption},
+ options::{
+ BtrfsBootdiskOptions, BtrfsCompressOption, Disk, FsType, ZfsBootdiskOptions,
+ ZfsChecksumOption, ZfsCompressOption,
+ },
utils::CidrAddress,
};
diff --git a/proxmox-tui-installer/src/views/mod.rs b/proxmox-tui-installer/src/views/mod.rs
index 673905f..b028543 100644
--- a/proxmox-tui-installer/src/views/mod.rs
+++ b/proxmox-tui-installer/src/views/mod.rs
@@ -182,7 +182,7 @@ impl<T: Copy + ToString + FromStr + PartialOrd> NumericEditView<T> {
///
/// # Arguments
/// * `content` - New, stringified content for the inner [`EditView`]. Must be a valid value
- /// according to the containet type `T`.
+ /// according to the container type `T`.
fn content_inner(mut self, content: &str) -> Self {
let mut inner = EditView::new();
std::mem::swap(self.inner_mut(), &mut inner);
--
2.47.0
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 4+ messages in thread* [pve-devel] [PATCH installer v3 2/2] assistant: avoid regex for simple prefix matching
2024-11-14 14:15 [pve-devel] [PATCH installer v3 0/2] assistant: clean up glob patterns & regexes Christoph Heiss
2024-11-14 14:15 ` [pve-devel] [PATCH installer v3 1/2] tree-wide: run rustfmt, fix clippy warnings Christoph Heiss
@ 2024-11-14 14:15 ` Christoph Heiss
2024-11-14 19:53 ` [pve-devel] applied: [PATCH installer v3 0/2] assistant: clean up glob patterns & regexes Thomas Lamprecht
2 siblings, 0 replies; 4+ messages in thread
From: Christoph Heiss @ 2024-11-14 14:15 UTC (permalink / raw)
To: pve-devel
udev properties are very easy to parse and can be done by doing a
line-based scan and matching the prefix, splitting once for properties.
Avoids the use of regexes and signicantly reduces binary size by about
-46%(!).
Tested by comparing the output of `proxmox-auto-install-assistant
device-info`, running it before and after the changes.
Stripped binary size for release builds:
before: 3869304 bytes ~ 3.69MiB
after: 2091608 bytes ~ 1.99MiB
text data bss dec hex filename
3580692 280920 545 3862157 3aee8d assistant-before
2031252 52336 505 2084093 1fccfd assistant-after
No functional changes.
Reviewed-By: Aaron Lauterer <a.lauterer@proxmox.com>
Tested-By: Aaron Lauterer <a.lauterer@proxmox.com>
Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
Changes v1 -> v2:
* rebased on latest master
Changes v2 -> v3:
* rebased on latest master
proxmox-auto-install-assistant/src/main.rs | 57 +++++++++-------------
1 file changed, 24 insertions(+), 33 deletions(-)
diff --git a/proxmox-auto-install-assistant/src/main.rs b/proxmox-auto-install-assistant/src/main.rs
index da1ebda..bdcf067 100644
--- a/proxmox-auto-install-assistant/src/main.rs
+++ b/proxmox-auto-install-assistant/src/main.rs
@@ -1,7 +1,6 @@
use anyhow::{bail, format_err, Result};
use clap::{Args, Parser, Subcommand, ValueEnum};
use glob::Pattern;
-use regex::Regex;
use serde::Serialize;
use std::{
collections::BTreeMap,
@@ -454,13 +453,9 @@ fn get_disks() -> Result<BTreeMap<String, BTreeMap<String, String>>> {
Pattern::new("sr[0-9]*")?,
];
- // compile Regex here once and not inside the loop
- let re_disk = Regex::new(r"(?m)^E: DEVTYPE=disk")?;
- let re_cdrom = Regex::new(r"(?m)^E: ID_CDROM")?;
- let re_iso9660 = Regex::new(r"(?m)^E: ID_FS_TYPE=iso9660")?;
-
- let re_name = Regex::new(r"(?m)^N: (.*)$")?;
- let re_props = Regex::new(r"(?m)^E: ([^=]+)=(.*)$")?;
+ const PROP_DEVTYP_PREFIX: &str = "E: DEVTYPE=";
+ const PROP_CDROM: &str = "E: ID_CDROM";
+ const PROP_ISO9660_FS: &str = "E: ID_FS_TYPE=iso9660";
let mut disks: BTreeMap<String, BTreeMap<String, String>> = BTreeMap::new();
@@ -482,30 +477,27 @@ fn get_disks() -> Result<BTreeMap<String, BTreeMap<String, String>>> {
}
};
- if !re_disk.is_match(&output) {
- continue 'outer;
- };
- if re_cdrom.is_match(&output) {
- continue 'outer;
- };
- if re_iso9660.is_match(&output) {
- continue 'outer;
- };
-
let mut name = filename;
- if let Some(cap) = re_name.captures(&output) {
- if let Some(res) = cap.get(1) {
- name = String::from(res.as_str());
+ let mut udev_props: BTreeMap<String, String> = BTreeMap::new();
+ for line in output.lines() {
+ if let Some(prop) = line.strip_prefix(PROP_DEVTYP_PREFIX) {
+ if prop != "disk" {
+ continue 'outer;
+ }
}
- }
- let mut udev_props: BTreeMap<String, String> = BTreeMap::new();
+ if line.starts_with(PROP_CDROM) || line.starts_with(PROP_ISO9660_FS) {
+ continue 'outer;
+ }
- for line in output.lines() {
- if let Some(caps) = re_props.captures(line) {
- let key = String::from(caps.get(1).unwrap().as_str());
- let value = String::from(caps.get(2).unwrap().as_str());
- udev_props.insert(key, value);
+ if let Some(prop) = line.strip_prefix("N: ") {
+ name = prop.to_owned();
+ };
+
+ if let Some(prop) = line.strip_prefix("E: ") {
+ if let Some((key, val)) = prop.split_once('=') {
+ udev_props.insert(key.to_owned(), val.to_owned());
+ }
}
}
@@ -515,7 +507,6 @@ fn get_disks() -> Result<BTreeMap<String, BTreeMap<String, String>>> {
}
fn get_nics() -> Result<BTreeMap<String, BTreeMap<String, String>>> {
- let re_props = Regex::new(r"(?m)^E: (.*)=(.*)$")?;
let mut nics: BTreeMap<String, BTreeMap<String, String>> = BTreeMap::new();
let links = get_nic_list()?;
@@ -533,10 +524,10 @@ fn get_nics() -> Result<BTreeMap<String, BTreeMap<String, String>>> {
let mut udev_props: BTreeMap<String, String> = BTreeMap::new();
for line in output.lines() {
- if let Some(caps) = re_props.captures(line) {
- let key = String::from(caps.get(1).unwrap().as_str());
- let value = String::from(caps.get(2).unwrap().as_str());
- udev_props.insert(key, value);
+ if let Some(prop) = line.strip_prefix("E: ") {
+ if let Some((key, val)) = prop.split_once('=') {
+ udev_props.insert(key.to_owned(), val.to_owned());
+ }
}
}
--
2.47.0
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 4+ messages in thread* [pve-devel] applied: [PATCH installer v3 0/2] assistant: clean up glob patterns & regexes
2024-11-14 14:15 [pve-devel] [PATCH installer v3 0/2] assistant: clean up glob patterns & regexes Christoph Heiss
2024-11-14 14:15 ` [pve-devel] [PATCH installer v3 1/2] tree-wide: run rustfmt, fix clippy warnings Christoph Heiss
2024-11-14 14:15 ` [pve-devel] [PATCH installer v3 2/2] assistant: avoid regex for simple prefix matching Christoph Heiss
@ 2024-11-14 19:53 ` Thomas Lamprecht
2 siblings, 0 replies; 4+ messages in thread
From: Thomas Lamprecht @ 2024-11-14 19:53 UTC (permalink / raw)
To: Proxmox VE development discussion, Christoph Heiss
Am 14.11.24 um 15:15 schrieb Christoph Heiss:
> The proxmox-auto-install-assistant uses regexes for udev property
> matching, which can be simplified by some simple prefix matching and
> splitting on '='.
>
> The latter also significantly reduces binary size (-46%) due to the
> removing the regex dependency, for details see patch #2.
>
> Overall no functional changes in this series.
>
> history
> =======
>
> v2: https://lore.proxmox.com/pve-devel/20240513094936.412650-1-c.heiss@proxmox.com/
> v1: https://lists.proxmox.com/pipermail/pve-devel/2024-May/063802.html
>
> Changes v2 -> v3:
> * rebased on latest master, dropping applied & obsolete patches
>
> Changes v1 -> v2:
> * rebased on latest master
>
> Christoph Heiss (2):
> tree-wide: run rustfmt, fix clippy warnings
> assistant: avoid regex for simple prefix matching
>
> proxmox-auto-install-assistant/src/main.rs | 61 +++++++++-----------
> proxmox-auto-installer/tests/parse-answer.rs | 2 +-
> proxmox-installer-common/src/setup.rs | 5 +-
> proxmox-tui-installer/src/views/mod.rs | 2 +-
> 4 files changed, 32 insertions(+), 38 deletions(-)
>
applied both patches, thanks!
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 4+ messages in thread