Uh oh!
There was an error while loading. Please reload this page.
Fix default send latency - #173
Conversation
There was a problem hiding this comment.
Pull request overview
This PR fixes a bug in the send latency calculation where the default send latency value was being incorrectly multiplied by 1000. The PR description states that send_lat is already in nanoseconds on the default path and should not be multiplied.
Changes:
- Moved the multiplication by 1000 to only occur when the AS_SEND_LAT environment variable is set (converting from microseconds to nanoseconds)
- Removed the unconditional multiplication that was incorrectly applied to both the default value and environment variable values
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| send_lat = 1000 * std::stoi(send_lat_env); | ||
| } catch (const std::invalid_argument& e) { | ||
| NcclLog->writeLog(NcclLogLevel::ERROR,"send_lat set error"); | ||
| exit(-1); |
There was a problem hiding this comment.
The exception handling for std::stoi only catches std::invalid_argument but not std::out_of_range, which can also be thrown when the converted value would fall out of the range of int. Consider catching both exceptions or using a catch-all for std::exception to handle both cases consistently.
| exit(-1); | |
| exit(-1); | |
| } catch (conststd::out_of_range&e) { | |
| NcclLog->writeLog(NcclLogLevel::ERROR,"send_lat set error"); | |
| exit(-1); |
Do not multiply
send_latby 1000 on default path as it's already ns there.