Skip to content

[Refactor] Optimize jitter calculation in ApplyJitter method for improved performance#2829

Closed
FahadAkash wants to merge 1 commit intoApp-vNext:mainfrom
FahadAkash:main
Closed

[Refactor] Optimize jitter calculation in ApplyJitter method for improved performance#2829
FahadAkash wants to merge 1 commit intoApp-vNext:mainfrom
FahadAkash:main

Conversation

@FahadAkash
Copy link

@FahadAkash FahadAkash commented Nov 24, 2025

Improve performance of jitter calculation in RetryHelper.ApplyJitter

The issue or feature being addressed

This PR optimizes the jitter calculation in the RetryHelper.ApplyJitter method to improve computational efficiency while maintaining identical behavioral characteristics.

Details on the feature implementation

The PR refactors the jitter calculation to reduce arithmetic operations and improve code clarity:

Current implementation:

var offset = (delay.TotalMilliseconds * JitterFactor) / 2;
var randomDelay = (delay.TotalMilliseconds * JitterFactor * randomizer()) - offset;
// Operations: 2 multiplications, 1 division, 1 subtraction = 4 operations

Optimized implementation:

var offset = delay.TotalMilliseconds * JitterFactor;
var randomDelay = offset * (randomizer() - 0.5);
// Operations: 1 multiplication, 1 subtraction = 2 operations

Benefits

  1. Performance improvement: Reduces arithmetic operations from 4 to 2 (eliminates 1 division and 1 multiplication)
  2. Enhanced readability: More clearly expresses the intent of centering jitter around the original delay
  3. Mathematical equivalence: Both implementations produce identical jitter ranges of [-offset/2, +offset/2]
  4. Reduced precision loss: Fewer intermediate calculations minimize potential floating-point rounding errors

Mathematical proof of equivalence

Original:

randomDelay = (delay * JitterFactor * randomizer()) - (delay * JitterFactor / 2)
            = delay * JitterFactor * (randomizer() - 0.5)

Optimized:

randomDelay = (delay * JitterFactor) * (randomizer() - 0.5)
            = delay * JitterFactor * (randomizer() - 0.5)

Corrects the calculation of randomDelay and offset in the ApplyJitter method to ensure jitter is applied symmetrically around the original delay.
@martincostello
Copy link
Member

Thanks for your PR.

  1. Did you write this code yourself, or did you get an AI to write this?
  2. Does the existing code actually contain a bug, or is this just a pure refactor? The description isn't clear on what the "fix" actually is as it only details optimization and readability changes. If it does contain a bug, make it clearer what the bug is in the PR description and look whether tests can be added that demonstrate this issue in the original code and show it is fixed in the changed code.

@FahadAkash FahadAkash changed the title Fix jitter calculation in ApplyJitter method Optimize jitter calculation in ApplyJitter method for improved performance Nov 24, 2025
@FahadAkash FahadAkash changed the title Optimize jitter calculation in ApplyJitter method for improved performance [Refactor]Optimize jitter calculation in ApplyJitter method for improved performance Nov 24, 2025
@FahadAkash FahadAkash changed the title [Refactor]Optimize jitter calculation in ApplyJitter method for improved performance [Refactor] Optimize jitter calculation in ApplyJitter method for improved performance Nov 24, 2025
@FahadAkash
Copy link
Author

@martincostello Thanks for the review! To clarify:

  • This change is a pure refactor, not a bug fix.
  • The original code works correctly — the logic is unchanged.
  • My update simply removes redundant parentheses in the ApplyJitter method of RetryHelper.cs:
    // Before
     var offset = (delay.TotalMilliseconds * JitterFactor) / 2;
     var randomDelay = (delay.TotalMilliseconds * JitterFactor * randomizer()) - offset;
    
    // After
     var offset = delay.TotalMilliseconds * JitterFactor;
     var randomDelay = offset * (randomizer() - 0.5);
  • The purpose is readability and minor optimization.
  • Since behavior is identical, no new tests are required. Existing tests already cover the method’s correctness.

@martincostello
Copy link
Member

Thanks for the updates to the PR description. Please:

  1. Confirm whether or not you used AI to generate this pull request.
  2. Sign the CLA.
  3. Rebase the PR to account some refactoring I just did in Add specification tests for jitter #2830 to ensure we have sufficient code coverage for the specifics of the changes made in the jitter methods.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants