(Server)DeviceFeature cleanup and optimization#852
(Server)DeviceFeature cleanup and optimization#852jsmnbom wants to merge 5 commits intobuttplugio:devfrom
Conversation
|
Once again part of #833, focused on memory improvements, but also took time to do general usability improvements. |
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Complexity | 57 |
| Duplication | -19 |
TIP This summary will be updated as you push new changes. Give us feedback
|
Heads up, I had to rebase dev to notch out the lovense remote/connect removals (which need to wait until a minor version update, and I need to do a bugfix version like, now). This makes your PR look very weird, but rest assured it's 100% my fault. Recommended steps here if you want to clean up (but read rest of message before starting):
This is really only if you've got more changes coming into this, otherwise after I get this next version released I plan on addressing this PR so I can handle the branch fuckery otherwise. |
|
I don't think I got any more changes that should be in this PR so I'll let you handle that ^^ |
|
Actually, I think I have an improvement. Lemme try ^^ |
- Simplified device output filtering in ButtplugClientDevice by using `values()` and `contains_output()`. - Updated input availability check to utilize `values()` and `contains_input()`. - Refactored input feature retrieval to streamline the process using `values()` and `contains_input()`. - Enhanced `DeviceFeature` to use `SmallVecEnumMap` for output and input features, improving memory efficiency and serialization. - Introduced `RangeInclusive` utility for better range handling. - Added custom serializers for `BitFlags<T>` to maintain wire format consistency. - Implemented `SmallVecEnumMap` for compact enum storage and serialization. - Updated dependencies in Cargo.toml to include `smallvec` and `enumflags2`.
… and performance - Consolidated range handling in RangeWithLimit struct, removing unnecessary internal_base field. - Simplified serialization and deserialization for RangeWithLimit. - Replaced Option types with SmallVecEnumMap for output and input properties in ServerDeviceFeature for better performance and memory efficiency. - Updated ServerDeviceFeatureOutput and ServerDeviceFeatureInput to use enum discriminants for cleaner variant handling. - Enhanced methods for checking and retrieving outputs and inputs, improving usability. - Removed redundant code and improved overall structure for better maintainability.
| } | ||
| } | ||
|
|
||
| impl From<ServerDeviceFeatureOutput> for DeviceFeatureOutput { |
There was a problem hiding this comment.
Not sure the new logic of filtering disabled is present anymore after the cherry pick stuff - may need to double check
|
Nevermind, I thought I had a good idea where I would change |
Refactors
DeviceFeatureandServerDeviceFeatureto replace structs with manyOptionfields withSmallVecEnumMap— a compact,SmallVec-backed collection that serializes/deserializes as a JSON object. While memory optimization was the primary goal, I took the opportunity to do a general cleanup of the surrounding code, keeping the scope contained and small. This dramatically reduces struct sizes while preserving the existing wire format and config file format.Key Changes
Memory & Performance
SmallVecEnumMap— New utility (buttplug_core::util::small_vec_enum_map) that stores enum variants in aSmallVecand round-trips through a JSON object ({"vibrate": {...}, "rotate": {...}}). Optimized for the common case of one output and maybe one input (N = 1means zero heap allocations). Falls back to heap allocation when more entries are present. The serde code is large but straightforward overall.DeviceFeature: 584 → 120 bytesServerDeviceFeature: 904 → 184 bytesRangeInclusive(buttplug_core::util::range) — Smaller thanstd::ops::RangeInclusiveand serializes cleanly without scattered serde helpers. Removesrange_serialize.rsand inlinerange_vec_serdemodules.API & Ergonomics
EnumDiscriminantsforOutputType/InputType— DerivesServerOutputTypeandServerInputTypeinternally. Adding a variant to one enum but not the other becomes a compile-time error.getsetusage —getsetis retained inbuttplug_coreandbuttplug_clientwhere structs are exposed to end users, but reduced from internal structs where it is only used to preserve encapsulation and invariants. Each proc-macro adds compile time, so trimming it down helps.Option/map access patterns with helper methods likecontains_output,contains_input,get_output_limits, andget_input, reducing branching and repeated unwrap/lookup code.examples/src/bin/device_control.rsandexamples/src/bin/device_tester.rswere updated to use the new helper-driven access pattern.Serialization
SmallVecEnumMapserializes as a JSON object keyed by variant name, matching the previous struct-with-Option-fields format exactly.RangeWithLimitnow uses#[serde(transparent)]instead of customSerialize/Deserializeimplementations.HashSet<InputCommandType>toBitFlags<InputCommandType>, serialized via a dedicated helper as the same sequence-of-command-names shape as before.Dependencies
smallvec(withserdeandconst_generics) inbuttplug_corefor compact inline storage.enumflags2inbuttplug_coreandbuttplug_server_device_configfor flag-style command storage.Validation & Scope
crates/buttplug_tests/tests/util/device_test/client/client_v4/mod.rs.rustfmtto changed files.Potential Concerns
OutputType. The serialization path prevents this from occurring in practice, but it's worth noting as a difference from the previous struct-based approach where each field was a distinctOption.