-
Notifications
You must be signed in to change notification settings - Fork 74
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Cleanup exponential timer #42
Conversation
- So that it’s symbol actual belongs to SPTDataLoader and won’t collide with some other module’s symbol. - Adds the `r` at the end since it’s described by its documentation as a timer. Not a specific time. - Renames the object creation methods as well to match.
- `currentTime` was just an internal property which was re-exposed as `timeInterval`.
- To make it easier to understand.
return arc4random_uniform(EXPT_MODULO + 1); | ||
} | ||
|
||
+ (NSTimeInterval)normalWithMu:(double)mu sigma:(double)sigma |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn’t change mu
as I don’t know what it stands for. Or if it’s actually a term used by the algorithm mentioned in the comment below.
Looks like there wasn't a proper file move (or it got clobbered somehow). Here's the diff of 21c21
< #import "SPTExpTime.h"
---
> #import "SPTDataLoaderExponentialTimer.h"
26,27d25
< static const double kDefaultGrow = M_E;
< const double kDefaultJitter = 0.11304999836;
29c27
< @interface SPTExpTime ()
---
> #pragma mark - Default Values
31,35c29,42
< @property (nonatomic, assign) NSTimeInterval currTime;
< @property (nonatomic, assign) double growFactor;
< @property (nonatomic, assign) NSTimeInterval maxTime;
< @property (nonatomic, assign) double jitter;
< @property (nonatomic, assign) NSTimeInterval initialTime;
---
> static const double SPTDataLoaderExponentialTimerDefaultGrow = M_E;
> const double SPTDataLoaderExponentialTimerDefaultJitter = 0.11304999836;
>
>
> #pragma mark - SPTDataLoaderExponentialTimer Private Interface
>
> @interface SPTDataLoaderExponentialTimer ()
>
> @property (nonatomic, assign, readwrite) NSTimeInterval timeInterval;
> @property (nonatomic, assign, readonly) NSTimeInterval maxTime;
> @property (nonatomic, assign, readonly) NSTimeInterval initialTime;
>
> @property (nonatomic, assign, readonly) double jitter;
> @property (nonatomic, assign, readonly) double growFactor;
39,42d45
< @implementation SPTExpTime
< {
< double _prevSigma;
< }
44,50c47,51
< + (instancetype)expTimeWithInitialTime:(NSTimeInterval)time0
< maxTime:(NSTimeInterval)maxTime
< growFactor:(double)growFactor
< jitter:(double)jitter
< {
< return [[SPTExpTime alloc] initWithInitialTime:time0 maxTime:maxTime growFactor:growFactor jitter:jitter];
< }
---
> #pragma mark - SPTDataLoaderExponentialTimer Implementation
>
> @implementation SPTDataLoaderExponentialTimer
>
> #pragma mark Creating an Exponential Timer Object
52c53,54
< + (instancetype)expTimeWithInitialTime:(NSTimeInterval)time0 maxTime:(NSTimeInterval)maxTime
---
> + (instancetype)exponentialTimerWithInitialTime:(NSTimeInterval)initialTime
> maxTime:(NSTimeInterval)maxTime
54c56
< return [[SPTExpTime alloc] initWithInitialTime:time0 maxTime:maxTime growFactor:kDefaultGrow jitter:0.0];
---
> return [self exponentialTimerWithInitialTime:initialTime maxTime:maxTime jitter:SPTDataLoaderExponentialTimerDefaultJitter];
57c59,61
< + (instancetype)expTimeWithInitialTime:(NSTimeInterval)time0 maxTime:(NSTimeInterval)maxTime jitter:(double)jitter
---
> + (instancetype)exponentialTimerWithInitialTime:(NSTimeInterval)initialTime
> maxTime:(NSTimeInterval)maxTime
> jitter:(double)jitter
59c63,66
< return [[SPTExpTime alloc] initWithInitialTime:time0 maxTime:maxTime growFactor:kDefaultGrow jitter:jitter];
---
> return [[self alloc] initWithInitialTime:initialTime
> maxTime:maxTime
> growFactor:SPTDataLoaderExponentialTimerDefaultGrow
> jitter:jitter];
62c69
< - (instancetype)initWithInitialTime:(NSTimeInterval)time0
---
> - (instancetype)initWithInitialTime:(NSTimeInterval)initialTime
72,73c79,80
< _initialTime = time0;
< _currTime = time0;
---
> _initialTime = initialTime;
> _timeInterval = initialTime;
80a88,94
> #pragma mark Accessing and Updating the Delay Value
>
> - (void)reset
> {
> self.timeInterval = self.initialTime;
> }
>
83c97
< NSTimeInterval t = self.currTime * self.growFactor;
---
> NSTimeInterval nextTime = self.timeInterval * self.growFactor;
85,86c99,100
< if (t > self.maxTime) {
< t = self.maxTime;
---
> if (nextTime > self.maxTime) {
> nextTime = self.maxTime;
90,94c104,107
< self.currTime = t;
< }
< else {
< const double sigma = self.jitter * t;
< self.currTime = [[self class] normalWithMu:t sigma:sigma];
---
> self.timeInterval = nextTime;
> } else {
> const double sigma = self.jitter * nextTime;
> self.timeInterval = [self.class normalWithMu:nextTime sigma:sigma];
97,98c110,111
< if (self.currTime > self.maxTime) {
< self.currTime = self.maxTime;
---
> if (self.timeInterval > self.maxTime) {
> self.timeInterval = self.maxTime;
101c114
< return self.currTime;
---
> return self.timeInterval;
106c119
< const NSTimeInterval ret = self.currTime;
---
> const NSTimeInterval timeInterval = self.timeInterval;
108,109d120
< return ret;
< }
111,113c122
< - (NSTimeInterval)timeInterval
< {
< return self.currTime;
---
> return timeInterval;
115a125,126
> #pragma mark Calculating Exponential Backoff
>
118c129,132
< #define exptRandom() (arc4random_uniform(EXPT_MODULO + 1))
---
> NS_INLINE double SPTExptRandom()
> {
> return arc4random_uniform(EXPT_MODULO + 1);
> }
128,132c142,146
< for (unsigned i = 0; i < 20; ++i) // Try 20 times
< {
< const double a = ((double)exptRandom()) / EXPT_MODULO_F64;
< const double b = 1.0 - (((double)exptRandom()) / EXPT_MODULO_F64);
< // const static float NV_MAGICCONST = 1.7155277699214135; //4 * exp(-0.5)/sqrt(2.0);
---
>
> const int attempts = 20;
> for (int i = 0; i < attempts; ++i) {
> const double a = SPTExptRandom() / EXPT_MODULO_F64;
> const double b = 1.0 - (SPTExptRandom() / EXPT_MODULO_F64);
136c150
< if (d <= -1.0*log(b)) {
---
> if (d <= -1.0 * log(b)) {
141,146c155
< return mu + 2.0 * sigma * (((double)exptRandom()) / EXPT_MODULO_F64);
< }
<
< - (void)reset
< {
< self.currTime = self.initialTime;
---
> return mu + 2.0 * sigma * (SPTExptRandom() / EXPT_MODULO_F64); |
Looks ok to me 👍 |
👍 Excellent work |
This PR cleans the exponential timer class used for back-off. So that it complies with Spotify’s code style guidelines. It also improves the class’ API documentation as well as removing duplicate and unused methods.
The changes to the files are so large that git does not manage to detect the rename. As such the diff is quite a lot larger than what it actually is.
@8W9aG @jgavris @dflems