Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Commit b23934a

Browse filesBrowse files
authored
Merge pull request hub4j#1134 from bitwiseman/task/atomic
Minimize locking for rate limit
2 parents f531096 + f2eecc3 commit b23934a
Copy full SHA for b23934a

File tree

Expand file treeCollapse file tree

2 files changed

+27
-26
lines changed
Filter options
Expand file treeCollapse file tree

2 files changed

+27
-26
lines changed

‎src/main/java/org/kohsuke/github/GHRateLimit.java

Copy file name to clipboardExpand all lines: src/main/java/org/kohsuke/github/GHRateLimit.java
+10-7Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
import java.time.format.DateTimeParseException;
1313
import java.util.Date;
1414
import java.util.Objects;
15+
import java.util.concurrent.atomic.AtomicReference;
1516
import java.util.logging.Logger;
1617

1718
import javax.annotation.CheckForNull;
@@ -344,7 +345,7 @@ public static class UnknownLimitRecord extends Record {
344345
private static final UnknownLimitRecord DEFAULT = new UnknownLimitRecord(Long.MIN_VALUE);
345346

346347
// The starting current UnknownLimitRecord is an expired record.
347-
private static UnknownLimitRecord current = DEFAULT;
348+
private static final AtomicReference<UnknownLimitRecord> current = new AtomicReference<>(DEFAULT);
348349

349350
/**
350351
* Create a new unknown record that resets at the specified time.
@@ -356,18 +357,20 @@ private UnknownLimitRecord(long resetEpochSeconds) {
356357
super(unknownLimit, unknownRemaining, resetEpochSeconds);
357358
}
358359

359-
static synchronized Record current() {
360-
if (current.isExpired()) {
361-
current = new UnknownLimitRecord(System.currentTimeMillis() / 1000L + unknownLimitResetSeconds);
360+
static Record current() {
361+
Record result = current.get();
362+
if (result.isExpired()) {
363+
current.set(new UnknownLimitRecord(System.currentTimeMillis() / 1000L + unknownLimitResetSeconds));
364+
result = current.get();
362365
}
363-
return current;
366+
return result;
364367
}
365368

366369
/**
367370
* Reset the current UnknownLimitRecord. For use during testing only.
368371
*/
369-
static synchronized void reset() {
370-
current = DEFAULT;
372+
static void reset() {
373+
current.set(DEFAULT);
371374
unknownLimitResetSeconds = defaultUnknownLimitResetSeconds;
372375
}
373376
}

‎src/main/java/org/kohsuke/github/GitHubClient.java

Copy file name to clipboardExpand all lines: src/main/java/org/kohsuke/github/GitHubClient.java
+17-19Lines changed: 17 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
import java.time.format.DateTimeFormatter;
1515
import java.time.temporal.ChronoUnit;
1616
import java.util.*;
17+
import java.util.concurrent.atomic.AtomicReference;
1718
import java.util.function.Consumer;
1819
import java.util.logging.Logger;
1920

@@ -52,10 +53,8 @@ abstract class GitHubClient {
5253

5354
private HttpConnector connector;
5455

55-
private final Object rateLimitLock = new Object();
56-
5756
@Nonnull
58-
private GHRateLimit rateLimit = GHRateLimit.DEFAULT;
57+
private final AtomicReference<GHRateLimit> rateLimit = new AtomicReference<>(GHRateLimit.DEFAULT);
5958

6059
private static final Logger LOGGER = Logger.getLogger(GitHubClient.class.getName());
6160

@@ -255,9 +254,7 @@ GHRateLimit getRateLimit(@Nonnull RateLimitTarget rateLimitTarget) throws IOExce
255254
@Nonnull
256255
@Deprecated
257256
GHRateLimit lastRateLimit() {
258-
synchronized (rateLimitLock) {
259-
return rateLimit;
260-
}
257+
return rateLimit.get();
261258
}
262259

263260
/**
@@ -277,12 +274,19 @@ GHRateLimit lastRateLimit() {
277274
*/
278275
@Nonnull
279276
GHRateLimit rateLimit(@Nonnull RateLimitTarget rateLimitTarget) throws IOException {
280-
synchronized (rateLimitLock) {
281-
if (rateLimit.getRecord(rateLimitTarget).isExpired()) {
282-
getRateLimit(rateLimitTarget);
277+
GHRateLimit result = rateLimit.get();
278+
// Most of the time rate limit is not expired, so try to avoid locking.
279+
if (result.getRecord(rateLimitTarget).isExpired()) {
280+
// if the rate limit is expired, synchronize to ensure
281+
// only one call to getRateLimit() is made to refresh it.
282+
synchronized (this) {
283+
if (rateLimit.get().getRecord(rateLimitTarget).isExpired()) {
284+
getRateLimit(rateLimitTarget);
285+
}
283286
}
284-
return rateLimit;
287+
result = rateLimit.get();
285288
}
289+
return result;
286290
}
287291

288292
/**
@@ -295,15 +299,9 @@ GHRateLimit rateLimit(@Nonnull RateLimitTarget rateLimitTarget) throws IOExcepti
295299
* {@link GHRateLimit.Record} constructed from the response header information
296300
*/
297301
private GHRateLimit updateRateLimit(@Nonnull GHRateLimit observed) {
298-
synchronized (rateLimitLock) {
299-
observed = rateLimit.getMergedRateLimit(observed);
300-
301-
if (rateLimit != observed) {
302-
rateLimit = observed;
303-
LOGGER.log(FINE, "Rate limit now: {0}", rateLimit);
304-
}
305-
return rateLimit;
306-
}
302+
GHRateLimit result = rateLimit.accumulateAndGet(observed, (current, x) -> current.getMergedRateLimit(x));
303+
LOGGER.log(FINEST, "Rate limit now: {0}", rateLimit.get());
304+
return result;
307305
}
308306

309307
/**

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.