PMEM: Optimization of enabled namespace methods #5873
Conversation
ut003460
commented
Feb 28, 2024
ut003460
commented
- Optimized the string connection section. Optimize repeated string concatenation operations into one-time concatenation instead of appending strings each time
- Replace subprocess.check_call with os.system to ensure that the code maintains its original functionality while improving execution efficiency to a certain extent
There was a problem hiding this comment.
Hi @ut003460, thank you for this. I wrote a few comments here, but my main concern is about changing the process.system to os.system. Can you please provide me more info about why this change is needed, because maybe I am missing something here? Thank you
1. Optimized the string connection section. Optimize repeated string concatenation operations into one-time concatenation instead of appending strings each time 2. Replace subprocess.check_call with os.system to ensure that the code maintains its original functionality while improving execution efficiency to a certain extent Signed-off-by: ut003460 <xiongneng@uniontech.com>
| @@ -265,12 +265,10 @@ def enable_region(self, name="all"): | |||
| def disable_namespace(self, namespace="all", region="", bus="", verbose=False): | |||
| """ | |||
| Disable namespaces | |||
|
|
|||
There was a problem hiding this comment.
Can you please return the new lines into this docstring. It improves the readability, and it will be in respect to PEP 257
| :param namespace: name of the namespace to be disabled | ||
| :param region: Filter namespace by region | ||
| :param bus: Filter namespace by bus | ||
| :param verbose: Enable True command with debug information | ||
|
|
| if bus: | ||
| args = f"{args} -b {bus}" | ||
| args += f" -b {bus}" |
There was a problem hiding this comment.
Actually, you're adding on operation here. Initially you have one "f string" operation (with two variables). Now you have both, one "f string" and a string concatenation.
I'm sorry but this is not an optimization.
Even worse, there's a consensus around the fact that usually string concatenations are slower than "f string" operations.
There was a problem hiding this comment.
This is not a very good optimization, thank you for your comment
