[–] 5 points 7 months ago (4 children)

Welcome to Rust, I hope you enjoyed learning and using it :)

One major advice would be to run cargo clippy, which gives a lot of helpful hints. Apart from that I also have some feedback.

    40	struct VendorOuiData {
    41	    vendor_organization: String, // Vendor Organization

These comments should be doc-comments (///) so that cargo doc picks them up.

    49	fn fetch_oui_database(url_string: &String) -> Result<String, Box<dyn std::error::Error>> {

Not saying that your return type is bad, but in general you might be interested in crates thiserror and anyhow that simplify error handling.

    51	        .timeout(Duration::new(60, 0))

Duration::from_mins(1) would be much more readable.

    66	                panic!("{}", err);

You should almost never panic. Your function already returns a Result<>, so you should be using that for error handling.

    70	    } else {
    71	        // HTTP Status is not 200

Not Rust specific, but usually I recommend to handle errors first, and return from the function early. This way you reduce cognitive load on the reader, and reduce nesting of if-blocks.

   150	                let mut parts = line.splitn(2, '\t');

There's a convenient method to split in two substrings that you could have used:

let (mac_address, vendor_organization) = line.split_once('\t').unwrap_or(("", ""));
   166	                vendor_oui_data.vendor_address += line
   167	                    .trim()
   168	                    .split(' ')
   169	                    .filter(|s| !s.is_empty())
   170	                    .collect::<Vec<_>>()
   171	                    .join(" ")
   172	                    .as_str();

I would probably write a regular expression that replaces all contiguous whitespace with a single space instead.

   173	                vendor_oui_data.vendor_address.push('\n');

Aren't you trimming this new line off in line 181?

   181	        vendor_oui_data.vendor_address = vendor_oui_data.vendor_address.trim().to_string();

This is somewhat inefficient, since you'll be allocating and copying your string here.

  • source
  • [–] 13 points 7 months ago* (2 children)

    parse_oui_database takes in a file path as a &String that is used to open the file in a parsing function. IMO there are a number of problems here.

    First, you should almost never take in a &String as a function argument. This basically means you have a reference to a owned object. It forces an allocation of everything passed into the function only to take a reference of it. It excludes types that are simply &strs forcing the caller to convert them to a full String - which involves an allocation. The function should just taking in a &str as it is cheap to convert a String to a &str (cheaper to use than a &String as well as &String have a double redirection).

    Sometimes it might be even better might be to take in a impl<AsRef(str)> which means the function can take in anything that can be converted into a &str without the caller needing to do it directly. Though on larger functions like this that might not always be the best idea as it makes it generic and so will be monomorphised for every type you pass into it. This can bloat a binary if you do it on lots of large functions with lots of different input types. You can also get the best of both worlds with a generic wrapper function to a concrete implementation - so the large function has a concrete type &str and a warpper that takes a impl <AsRef<str>> and calls the inner function. Though in this case it is probably easier to just take in a &str and manually convert at all the one call sites.

    Second. String/&str are not the write types for paths. Those would be PathBuf and &Path which both work like String and &str (so all the above applies to these as well). These are generally better to use as paths in most OSs dont have to be unicode. Which means there are files (though very rarely) which cannot be represented as a String. This is why File::open takes in a AsRef<Path> which your function can also.

    Lastly. I would not conflate opening a file with parsing it. These should be two different functions. This makes the code a bit more flexible - you can get the file to parse from other sources. One big advantage to this would be for testing where you can just have the test data as strings in the test. It also makes the returned error type simpler as one function can deal with io errors and the other with parsing errors. And in this case you can just parse the file directly from the internet request rather than saving it to a file first (though there are other reasons you may or may not want to do this).

  • source
  • [–] 4 points 7 months ago (1 child)

    You panic a lot on errors in functions where you return a result. Almost all of these are due to input problems not programming logic errors. These really should be return from the functions as errors so higher up functions can handle them how they might need to rather than just imminently crashing the program. Really panics should only be used when the situation should never occur and if it does that indicates a bug in the program - or you are being lazy with small/test code/one off scripts (but in this case why return any errors at all, you might as well just .unwrap() on everything instead of ?.

  • source
  • [–] 2 points 7 months ago (1 child)

    This is a pet peeve of mine, but don't put main last. Its like opening a book to be greeted by a random chapter - probably one near the end and you have to hunt through it to find where the story actually starts, which is probably near the end.

    This is a IMO horribly hangup from languages that require you to declare something before you can use it. You don't need to do that in rust. So put your functions in order that makes sense to read them from top to bottom. This typically means main should be one of the first functions you see - as it is the entry point to the code. In other files this might be the main functions a user is expected to use first. Sometimes you might want to see some datastructures before that. But overall things should be ordered by how it makes sense to read them to make it easier to make sense of the program.

  • source
  • [–] 2 points 7 months ago* (1 child)

    Only skimmed quickly.

    • Four different hex representations for the same 3 bytes is extremely wasteful. You don't need to store more than a [u8; 3]. You can create a "mapped" struct with those representations if caching them is somehow useful for the use-case. And since we're micro-optimizing memory usage, Box<str> instead of String (for the two fields with actual string data) would be more efficient since those values are not meant to be mutable. You can then serialize to one of the many efficient binary formats around (e.g. borsh). This is not JavaScript, so you can oftentimes not be bound to JSON.
    • Your parsing code looks both unrobust (potentially fragile hard-coded assumptions) and unnecessarily complex. Something much simpler can probably be done with some trivial regex usage.
  • source
  • [–] 4 points 7 months ago

    I just scrolled through it, so no deep insights, but at first glance it looks pretty good.

    One thing I saw, and that probably every rust programmer has to learn in the beginning, is to use slices for function parameters whenever prossible. Pretty much every time you use fn foo(s: &String) you should use fn foo(s: &str) and the same goes for &Vec<T> -> &[T]. Calling these works still the same so just changing the function definitions should work.

    The advantage of this is, that now you can use every type that can be dereferenced as the slice, so you can now call foo like this foo("bar"); or with a String foo(&String::from("bar"));. The same applies for &[T] which can be called with &Vec<T> or an array &[T; N]

  • source
  • [–] 2 points 8 months ago

    My system is heating up

    A small note on that. If you could live with the compilation speed but it's annoying to have the CPU fans working all the time, you could address it by moving the CPU to a more power-efficient mode. Though that will only slow things down for you, and you're saying it's already slow and you're running out of RAM, so maybe heat/fans are not your biggest concern.

    Personally, I currently use intel CPUs. Old ones I could undervolt AND limit in watts, making them silent and reducing power usage by a third at the cost of ~5% performance. My current one doesn't support undervolting sadly, and the options for wattage limits are also almost fully gone. What I use now is:

    /etc/tmpfiles.d/cpu-prefer-powersafe.conf

    # Possible values: default performance balance_performance balance_power power
    w /sys/devices/system/cpu/cpu*/cpufreq/scaling_governor - - - - powersave
    

    Replacing "powersave" with "power" cuts the energy usage by half and makes the CPU fully silent at ~50 degrees... at the cost of 30% performance on this new CPU :(( Or keeping it as above, energy usage goes down somewhat (haven't measured precisely), CPU mostly silent at 50-60 degrees, performance down by 15%.

    OK probably I wrote waay too much 😅

  • source
  • [–] 1 point 8 months ago

    --no-default-features --features=foo,bar is fine. The harmful part is replacing established crates with smaller ones solely because of ze size.

    And the whole dance doesn't even do what many people think it does, as covered in some comments in that linked old thread.

    Note that I made that jerk thread when min-dependency-ing was sort of trending. A trend that was often (not always) as stupid and counter-productive as the other related(-ish) trend min-binary-sizing.

    Also note that the harmfulness of that trend went beyond bad ecosystem dynamics and dependants ending up with less quality/reliability. That semi-obsession also encouraged bad coding practices like pervasive use of dyn where it's not needed. Compromising idiomaticity, and removing zero-cost abstractions to win a faster compiling dependency prize!

    I will stop here, because I really don't want to write that blog post.

  • source
  • parent
  • context
  • [–] 1 point 8 months ago (1 child)

    "Small amount" is relative here. Not everybody needs to play Battlefield 6.

    For just programming, 8GB is way more than enough. Even my old work laptop had only 8GB and the issues didn't show up until I had multiple Office products + Teams + a browser open, not from any of my dev software.

  • source
  • parent
  • context
  • [–] 8 points 8 months ago
    • Use zram so swapping doesn't immediately slow things to a crawl.
    • Use cargo check, often. You don't need to always compile.
    • Add a release-dev profile that inherits release, use cranelift for codegen in it, and turn off lto.

    Otherwise, it would be useful to know what kind of system you're running, and how is the system load without any rust dev involvement. It would also be helpful to provide specifics. Your descriptions are very generic and could be entirely constructed from rust memes.

  • source
  • [–] 0 points 8 months ago (2 children)

    8 GB is a really small amount. Even phones have had that much RAM for several years. The average desktop I built in 2012 had 16 GB of RAM.

    Plenty of modern computers only come with a small amount of RAM, because most people only need a small amount, but 8 GB is still a small amount.

  • source
  • parent
  • context
  • [–] 5 points 8 months ago (1 child)

    Couldn't agree more here, 1k dependencies would take a while to build even on my 9950x3d if only due to linking.

    It seems to me like the the issue is the project is either too bloated, or large enough to justify a workstation build. Breaking it into smaller, independent parts would also help here.

  • source
  • parent
  • context
  • [–] 11 points 8 months ago (3 children)

    8GB is not a lot to work with. It mostly depends on what crates you work with (unfortunately by default rust statically links all code in a single final step and that can be consuming a lot of RAM) Modern editors consume a lot of RAM, and if you have rust-analyzer running it's going to use a lot more in addition to the editor.

    Tips:

    • trim down yout dependencies (check out cargo tree, cargo depgraph and the cargo build --timings ). Instead of enabling all features on crates, enable only what you need.
    • to free up disk space run cargo clean on the projects you are not currently working on, and regularly on your current project. If you change dependencies often it's going to use up a lot of GBs of disk space. You'll need to recompile all the dependencies every time you run it though, so don't do it too often.
    • fully close other programs if possible (browsers, chat apps, etc). Look at your task manager to see what's using up memory and kill as much as possible. Consider browsing from your phone instead, and using the PC browser just as needed.
    • emacs and vim have a very steep learning curve. I would suggest against that. If it's still too frustrating to code like this, you can disable the language server (rust-analyzer), but it's going to make coding harder. You'll loose edit suggestions and error highlighting. It's still possible to code without rust-analyzer but you'll need to run cargo check very often and read up method names on docs.rs a lot more.
  • source
  • [–] 1 point 8 months ago

    If you have the ability to recompile your system ( Gentoo, changing the optimization from -O2 to -Os, optimizing for size ), that may significantly free your system's resources for stuff.

    Also, if you CAN'T add RAM, can you speed your storage?

    Going from spinning-platter to SATA-SSD, or from SATA-SSD to NVMe, or from normal-NVMe to FAST-NVMe, would massively help your system's speed.

    _ /\ _

  • source
  • view more: ‹ prev next ›