Skip to content

Commit 24e572e

Browse files
gtheagthea
committed
Refactor'
1 parent c608293 commit 24e572e

File tree

10 files changed

+149
-103
lines changed

10 files changed

+149
-103
lines changed

‎src/main/java/io/split/android/client/SplitFactoryImpl.java‎

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -18,13 +18,11 @@
1818
import java.util.concurrent.ThreadPoolExecutor;
1919
import java.util.concurrent.TimeUnit;
2020
import java.util.concurrent.atomic.AtomicBoolean;
21-
import java.util.concurrent.atomic.AtomicReference;
2221
import java.util.concurrent.locks.ReentrantLock;
2322

2423
import io.split.android.android_client.BuildConfig;
2524
import io.split.android.client.api.Key;
2625
import io.split.android.client.common.CompressionUtilProvider;
27-
import io.split.android.client.dtos.TargetingRulesChange;
2826
import io.split.android.client.events.EventsManagerCoordinator;
2927
import io.split.android.client.factory.FactoryMonitor;
3028
import io.split.android.client.factory.FactoryMonitorImpl;
@@ -48,6 +46,7 @@
4846
import io.split.android.client.service.impressions.strategy.PeriodicTracker;
4947
import io.split.android.client.service.impressions.strategy.ProcessStrategy;
5048
import io.split.android.client.service.splits.SplitsSyncHelper;
49+
import io.split.android.client.service.splits.TargetingRulesCache;
5150
import io.split.android.client.service.sseclient.sseclient.StreamingComponents;
5251
import io.split.android.client.service.synchronizer.SyncManager;
5352
import io.split.android.client.service.synchronizer.Synchronizer;
@@ -105,8 +104,7 @@ public class SplitFactoryImpl implements SplitFactory {
105104
private final SplitTaskExecutor mSplitTaskExecutor;
106105
private final SplitClientConfig mConfig;
107106

108-
private AtomicReference<TargetingRulesChange> mCachedFetchRef = null;
109-
private ReentrantLock mCachedFetchLock = null;
107+
private TargetingRulesCache mTargetingRulesCache = null;
110108

111109
private final ExecutorService mInitExecutor = Executors.newFixedThreadPool(2);
112110

@@ -216,7 +214,7 @@ private SplitFactoryImpl(@NonNull String apiToken, @NonNull Key key, @NonNull Sp
216214
SplitTaskFactory splitTaskFactory = new SplitTaskFactoryImpl(
217215
config, splitApiFacade, mStorageContainer, splitsFilterQueryStringFromConfig,
218216
getFlagsSpec(testingConfig), mEventsManagerCoordinator, filters, flagSetsFilter,
219-
testingConfig, mCachedFetchRef, mCachedFetchLock);
217+
testingConfig, mTargetingRulesCache);
220218

221219
SplitSingleThreadTaskExecutor splitSingleThreadTaskExecutor = new SplitSingleThreadTaskExecutor();
222220
splitSingleThreadTaskExecutor.pause();
@@ -418,8 +416,7 @@ private static String getFlagsSpec(@Nullable TestingConfig testingConfig) {
418416
}
419417

420418
private void startFreshInstallPrefetch(@NonNull SplitApiFacade splitApiFacade, @NonNull String flagsSpec, long initializationStartTime) {
421-
mCachedFetchRef = new AtomicReference<>(null);
422-
mCachedFetchLock = new ReentrantLock();
419+
mTargetingRulesCache = new TargetingRulesCache();
423420

424421
Runnable prefetch = () -> {
425422
try {
@@ -429,8 +426,7 @@ private void startFreshInstallPrefetch(@NonNull SplitApiFacade splitApiFacade, @
429426
true,
430427
flagsSpec,
431428
splitApiFacade.getSplitFetcher(),
432-
mCachedFetchRef,
433-
mCachedFetchLock);
429+
mTargetingRulesCache);
434430
long elapsedTime = System.currentTimeMillis() - initializationStartTime;
435431
Logger.d("Fresh install prefetch completed in " + elapsedTime + "ms");
436432
} catch (HttpFetcherException e) {

‎src/main/java/io/split/android/client/service/executor/SplitTaskFactoryImpl.java‎

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -11,16 +11,13 @@
1111
import java.util.List;
1212
import java.util.Map;
1313
import java.util.Set;
14-
import java.util.concurrent.atomic.AtomicReference;
15-
import java.util.concurrent.locks.ReentrantLock;
1614

1715
import io.split.android.client.FlagSetsFilter;
1816
import io.split.android.client.SplitClientConfig;
1917
import io.split.android.client.SplitFilter;
2018
import io.split.android.client.TestingConfig;
2119
import io.split.android.client.dtos.RuleBasedSegment;
2220
import io.split.android.client.dtos.Split;
23-
import io.split.android.client.dtos.TargetingRulesChange;
2421
import io.split.android.client.events.ISplitEventsManager;
2522
import io.split.android.client.service.CleanUpDatabaseTask;
2623
import io.split.android.client.service.ServiceConstants;
@@ -46,6 +43,7 @@
4643
import io.split.android.client.service.splits.SplitsSyncHelper;
4744
import io.split.android.client.service.splits.SplitsSyncTask;
4845
import io.split.android.client.service.splits.SplitsUpdateTask;
46+
import io.split.android.client.service.splits.TargetingRulesCache;
4947
import io.split.android.client.service.sseclient.ReconnectBackoffCounter;
5048
import io.split.android.client.service.telemetry.TelemetryConfigRecorderTask;
5149
import io.split.android.client.service.telemetry.TelemetryStatsRecorderTask;
@@ -84,8 +82,7 @@ public SplitTaskFactoryImpl(@NonNull SplitClientConfig splitClientConfig,
8482
@Nullable Map<SplitFilter.Type, SplitFilter> filters,
8583
@Nullable FlagSetsFilter flagSetsFilter,
8684
@Nullable TestingConfig testingConfig,
87-
@Nullable AtomicReference<TargetingRulesChange> cacheRef,
88-
@Nullable ReentrantLock cachedFetchLock) {
85+
@Nullable TargetingRulesCache targetingRulesCache) {
8986

9087
mSplitClientConfig = checkNotNull(splitClientConfig);
9188
mSplitApiFacade = checkNotNull(splitApiFacade);
@@ -109,8 +106,7 @@ public SplitTaskFactoryImpl(@NonNull SplitClientConfig splitClientConfig,
109106
mTelemetryRuntimeProducer,
110107
new ReconnectBackoffCounter(1, testingConfig.getCdnBackoffTime()),
111108
flagsSpecFromConfig,
112-
cacheRef,
113-
cachedFetchLock);
109+
targetingRulesCache);
114110
} else {
115111
mSplitsSyncHelper = new SplitsSyncHelper(mSplitApiFacade.getSplitFetcher(),
116112
mSplitsStorageContainer.getSplitsStorage(),
@@ -121,8 +117,7 @@ public SplitTaskFactoryImpl(@NonNull SplitClientConfig splitClientConfig,
121117
mTelemetryRuntimeProducer,
122118
flagsSpecFromConfig,
123119
false,
124-
cacheRef,
125-
cachedFetchLock);
120+
targetingRulesCache);
126121
}
127122

128123
mFilters = (filters == null) ? new ArrayList<>() : new ArrayList<>(filters.values());

‎src/main/java/io/split/android/client/service/splits/SplitsSyncHelper.java‎

Lines changed: 35 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,6 @@
1313
import java.util.concurrent.ExecutorService;
1414
import java.util.concurrent.Executors;
1515
import java.util.concurrent.TimeUnit;
16-
import java.util.concurrent.atomic.AtomicReference;
17-
import java.util.concurrent.locks.ReentrantLock;
1816

1917
import io.split.android.client.dtos.RuleBasedSegmentChange;
2018
import io.split.android.client.dtos.SplitChange;
@@ -54,8 +52,7 @@ public class SplitsSyncHelper {
5452
private final BackoffCounter mBackoffCounter;
5553
private final OutdatedSplitProxyHandler mOutdatedSplitProxyHandler;
5654
private final ExecutorService mExecutor;
57-
private final AtomicReference<TargetingRulesChange> mCacheRef;
58-
private ReentrantLock mCachedFetchLock;
55+
private final TargetingRulesCache mTargetingRulesCache;
5956

6057
public SplitsSyncHelper(@NonNull HttpFetcher<TargetingRulesChange> splitFetcher,
6158
@NonNull SplitsStorage splitsStorage,
@@ -66,8 +63,7 @@ public SplitsSyncHelper(@NonNull HttpFetcher<TargetingRulesChange> splitFetcher,
6663
@NonNull TelemetryRuntimeProducer telemetryRuntimeProducer,
6764
@Nullable String flagsSpec,
6865
boolean forBackgroundSync,
69-
@Nullable AtomicReference<TargetingRulesChange> cacheRef,
70-
@Nullable ReentrantLock cachedFetchLock) {
66+
@Nullable TargetingRulesCache targetingRulesCache) {
7167
this(splitFetcher,
7268
splitsStorage,
7369
splitChangeProcessor,
@@ -79,8 +75,7 @@ public SplitsSyncHelper(@NonNull HttpFetcher<TargetingRulesChange> splitFetcher,
7975
flagsSpec,
8076
forBackgroundSync,
8177
DEFAULT_PROXY_CHECK_INTERVAL_MILLIS,
82-
cacheRef,
83-
cachedFetchLock);
78+
targetingRulesCache);
8479
}
8580

8681
public SplitsSyncHelper(@NonNull HttpFetcher<TargetingRulesChange> splitFetcher,
@@ -92,8 +87,7 @@ public SplitsSyncHelper(@NonNull HttpFetcher<TargetingRulesChange> splitFetcher,
9287
@NonNull TelemetryRuntimeProducer telemetryRuntimeProducer,
9388
@NonNull BackoffCounter backoffCounter,
9489
@Nullable String flagsSpec,
95-
@Nullable AtomicReference<TargetingRulesChange> cacheRef,
96-
@Nullable ReentrantLock cachedFetchLock) {
90+
@Nullable TargetingRulesCache targetingRulesCache) {
9791
this(splitFetcher,
9892
splitsStorage,
9993
splitChangeProcessor,
@@ -105,8 +99,7 @@ public SplitsSyncHelper(@NonNull HttpFetcher<TargetingRulesChange> splitFetcher,
10599
flagsSpec,
106100
false,
107101
DEFAULT_PROXY_CHECK_INTERVAL_MILLIS,
108-
cacheRef,
109-
cachedFetchLock);
102+
targetingRulesCache);
110103
}
111104

112105
@VisibleForTesting
@@ -121,8 +114,7 @@ public SplitsSyncHelper(@NonNull HttpFetcher<TargetingRulesChange> splitFetcher,
121114
@Nullable String flagsSpec,
122115
boolean forBackgroundSync,
123116
long proxyCheckIntervalMillis,
124-
@Nullable AtomicReference<TargetingRulesChange> cacheRef,
125-
@Nullable ReentrantLock cachedFetchLock) {
117+
@Nullable TargetingRulesCache targetingRulesCache) {
126118
mSplitFetcher = checkNotNull(splitFetcher);
127119
mSplitsStorage = checkNotNull(splitsStorage);
128120
mSplitChangeProcessor = checkNotNull(splitChangeProcessor);
@@ -132,8 +124,7 @@ public SplitsSyncHelper(@NonNull HttpFetcher<TargetingRulesChange> splitFetcher,
132124
mBackoffCounter = checkNotNull(backoffCounter);
133125
mOutdatedSplitProxyHandler = new OutdatedSplitProxyHandler(flagsSpec, forBackgroundSync, generalInfoStorage, proxyCheckIntervalMillis);
134126
mExecutor = Executors.newSingleThreadExecutor();
135-
mCacheRef = cacheRef;
136-
mCachedFetchLock = cachedFetchLock;
127+
mTargetingRulesCache = targetingRulesCache;
137128
}
138129

139130
public SplitTaskExecutionInfo sync(SinceChangeNumbers till, int onDemandFetchBackoffMaxRetries) {
@@ -236,52 +227,39 @@ private CdnByPassType attemptSplitSync(SinceChangeNumbers targetChangeNumber, bo
236227

237228
private SinceChangeNumbers fetchUntil(SinceChangeNumbers till, boolean clearBeforeUpdate, boolean avoidCache, CdnByPassType withCdnByPass, boolean resetChangeNumber) throws Exception {
238229
boolean shouldClearBeforeUpdate = clearBeforeUpdate;
239-
boolean usedCache = false;
240230

241231
SinceChangeNumbers newTill = till;
242-
boolean lockAcquired = false;
243-
try {
244-
while (true) {
245-
if (mCachedFetchLock != null && !lockAcquired) {
246-
mCachedFetchLock.lock();
247-
lockAcquired = true;
248-
}
249-
long changeNumber = (resetChangeNumber) ? -1 : mSplitsStorage.getTill();
250-
long rbsChangeNumber = (resetChangeNumber) ? -1 : mRuleBasedSegmentStorage.getChangeNumber();
251-
resetChangeNumber = false;
252-
if ((newTill.getFlagsSince() < changeNumber) && ((newTill.getRbsSince() == null) || (newTill.getRbsSince() < rbsChangeNumber))) {
253-
return new SinceChangeNumbers(changeNumber, rbsChangeNumber);
254-
}
232+
while (true) {
233+
long changeNumber = (resetChangeNumber) ? -1 : mSplitsStorage.getTill();
234+
long rbsChangeNumber = (resetChangeNumber) ? -1 : mRuleBasedSegmentStorage.getChangeNumber();
235+
resetChangeNumber = false;
236+
if ((newTill.getFlagsSince() < changeNumber) && ((newTill.getRbsSince() == null) || (newTill.getRbsSince() < rbsChangeNumber))) {
237+
return new SinceChangeNumbers(changeNumber, rbsChangeNumber);
238+
}
255239

256-
TargetingRulesChange targetingRulesChange;
257-
if ((rbsChangeNumber == -1L) && changeNumber == -1 && mCacheRef != null && mCacheRef.get() != null) {
240+
TargetingRulesChange targetingRulesChange;
241+
// Try to use cached value for fresh installs
242+
if ((rbsChangeNumber == -1L) && changeNumber == -1 && mTargetingRulesCache != null) {
243+
targetingRulesChange = mTargetingRulesCache.getAndConsume();
244+
if (targetingRulesChange != null) {
258245
Logger.d("Fresh install: Using prefetched targeting rules from cache");
259-
targetingRulesChange = mCacheRef.get();
260-
usedCache = true;
261246
} else {
262247
Logger.d("Fetching targeting rules - changeNumber: " + changeNumber + ", rbsChangeNumber: " + rbsChangeNumber);
263248
targetingRulesChange = fetchSplits(new SinceChangeNumbers(changeNumber, rbsChangeNumber), avoidCache, withCdnByPass);
264249
}
250+
} else {
251+
Logger.d("Fetching targeting rules - changeNumber: " + changeNumber + ", rbsChangeNumber: " + rbsChangeNumber);
252+
targetingRulesChange = fetchSplits(new SinceChangeNumbers(changeNumber, rbsChangeNumber), avoidCache, withCdnByPass);
253+
}
265254

266-
// Release lock after first use of cache to avoid blocking subsequent syncs
267-
if (lockAcquired && usedCache) {
268-
mCachedFetchLock.unlock();
269-
lockAcquired = false;
270-
}
271-
272-
SplitChange splitChange = targetingRulesChange.getFeatureFlagsChange();
273-
RuleBasedSegmentChange ruleBasedSegmentChange = targetingRulesChange.getRuleBasedSegmentsChange();
274-
updateStorage(shouldClearBeforeUpdate, splitChange, ruleBasedSegmentChange);
275-
shouldClearBeforeUpdate = false;
255+
SplitChange splitChange = targetingRulesChange.getFeatureFlagsChange();
256+
RuleBasedSegmentChange ruleBasedSegmentChange = targetingRulesChange.getRuleBasedSegmentsChange();
257+
updateStorage(shouldClearBeforeUpdate, splitChange, ruleBasedSegmentChange);
258+
shouldClearBeforeUpdate = false;
276259

277-
newTill = new SinceChangeNumbers(splitChange.till, ruleBasedSegmentChange.getTill());
278-
if (splitChange.till == splitChange.since && ruleBasedSegmentChange.getTill() == ruleBasedSegmentChange.getSince()) {
279-
return new SinceChangeNumbers(splitChange.till, ruleBasedSegmentChange.getTill());
280-
}
281-
}
282-
} finally {
283-
if (lockAcquired) {
284-
mCachedFetchLock.unlock();
260+
newTill = new SinceChangeNumbers(splitChange.till, ruleBasedSegmentChange.getTill());
261+
if (splitChange.till == splitChange.since && ruleBasedSegmentChange.getTill() == ruleBasedSegmentChange.getSince()) {
262+
return new SinceChangeNumbers(splitChange.till, ruleBasedSegmentChange.getTill());
285263
}
286264
}
287265
}
@@ -310,9 +288,8 @@ public static void fetchSplits(SinceChangeNumbers till,
310288
boolean avoidCache,
311289
String currentSpec,
312290
HttpFetcher<TargetingRulesChange> fetcher,
313-
@NonNull AtomicReference<TargetingRulesChange> cacheRef,
314-
@NonNull ReentrantLock mCachedFetchLock) throws HttpFetcherException {
315-
mCachedFetchLock.lock();
291+
@NonNull TargetingRulesCache cache) throws HttpFetcherException {
292+
cache.lock();
316293
try {
317294
Map<String, Object> params = new LinkedHashMap<>();
318295
if (currentSpec != null && !currentSpec.trim().isEmpty()) {
@@ -321,10 +298,10 @@ public static void fetchSplits(SinceChangeNumbers till,
321298
params.put(SINCE_PARAM, till.getFlagsSince());
322299
params.put(RBS_SINCE_PARAM, till.getRbsSince());
323300

324-
TargetingRulesChange execute = fetcher.execute(params, getHeaders(avoidCache));
325-
cacheRef.set(execute);
301+
TargetingRulesChange result = fetcher.execute(params, getHeaders(avoidCache));
302+
cache.set(result);
326303
} finally {
327-
mCachedFetchLock.unlock();
304+
cache.unlock();
328305
}
329306
}
330307

‎src/main/java/io/split/android/client/service/splits/SplitsSyncTask.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ public static SplitTask buildForBackground(@NonNull SplitsSyncHelper splitsSyncH
4747
return new SplitsSyncTask(splitsSyncHelper, splitsStorage, ruleBasedSegmentStorage, splitsFilterQueryString, telemetryRuntimeProducer, null, 1);
4848
}
4949

50-
public SplitsSyncTask(@NonNull SplitsSyncHelper splitsSyncHelper,
50+
private SplitsSyncTask(@NonNull SplitsSyncHelper splitsSyncHelper,
5151
@NonNull SplitsStorage splitsStorage,
5252
@NonNull RuleBasedSegmentStorageProducer ruleBasedSegmentStorage,
5353
String splitsFilterQueryString,

‎src/main/java/io/split/android/client/service/splits/TargetingRulesCache.java‎

Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,90 @@
1+
package io.split.android.client.service.splits;
2+
3+
import androidx.annotation.Nullable;
4+
5+
import java.util.concurrent.locks.ReentrantLock;
6+
7+
import io.split.android.client.dtos.TargetingRulesChange;
8+
9+
/**
10+
* Thread-safe cache for storing prefetched targeting rules during fresh installs.
11+
* The cache is designed for single-use: once the cached value is consumed, it's cleared
12+
* and the lock is released to avoid blocking subsequent syncs.
13+
*/
14+
public class TargetingRulesCache {
15+
16+
private final ReentrantLock mLock;
17+
private volatile TargetingRulesChange mCachedValue;
18+
private volatile boolean mConsumed;
19+
20+
public TargetingRulesCache() {
21+
mLock = new ReentrantLock();
22+
mCachedValue = null;
23+
mConsumed = false;
24+
}
25+
26+
/**
27+
* Stores a value in the cache.
28+
*
29+
* @param value The targeting rules change to cache
30+
*/
31+
public void set(@Nullable TargetingRulesChange value) {
32+
mLock.lock();
33+
try {
34+
if (!mConsumed) {
35+
mCachedValue = value;
36+
}
37+
} finally {
38+
mLock.unlock();
39+
}
40+
}
41+
42+
/**
43+
* Retrieves and consumes the cached value if available.
44+
* After consumption, the cache is cleared and marked as consumed.
45+
*
46+
* @return The cached value, or null if not available or already consumed
47+
*/
48+
@Nullable
49+
public TargetingRulesChange getAndConsume() {
50+
if (mConsumed) {
51+
return null;
52+
}
53+
54+
mLock.lock();
55+
try {
56+
if (mConsumed || mCachedValue == null) {
57+
return null;
58+
}
59+
TargetingRulesChange result = mCachedValue;
60+
mCachedValue = null;
61+
mConsumed = true;
62+
return result;
63+
} finally {
64+
mLock.unlock();
65+
}
66+
}
67+
68+
/**
69+
* Acquires the lock for external operations. Must be paired with unlock().
70+
*/
71+
public void lock() {
72+
mLock.lock();
73+
}
74+
75+
/**
76+
* Releases the lock acquired by lock().
77+
*/
78+
public void unlock() {
79+
mLock.unlock();
80+
}
81+
82+
/**
83+
* Checks if a cached value is available (not yet consumed).
84+
*
85+
* @return true if a value is cached and not consumed
86+
*/
87+
public boolean hasValue() {
88+
return !mConsumed && mCachedValue != null;
89+
}
90+
}

0 commit comments

Comments
 (0)