Skip to content

Commit 91b6098

Browse files
poutsmapoutsma
committed
Do not use BitSet in BitsCronField
This commit changes BitsCronField to use a long instead of a BitSet, since the later can use significant memory. Closes gh-25687
1 parent dd011c9 commit 91b6098

File tree

4 files changed

+151
-123
lines changed

4 files changed

+151
-123
lines changed

‎spring-context/src/main/java/org/springframework/scheduling/support/BitsCronField.java

Lines changed: 68 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -34,28 +34,31 @@
3434
*/
3535
final class BitsCronField extends CronField {
3636

37-
private static final BitsCronField ZERO_NANOS;
37+
private static final long MASK = 0xFFFFFFFFFFFFFFFFL;
3838

3939

40-
static {
41-
ZERO_NANOS = new BitsCronField(Type.NANO);
42-
ZERO_NANOS.bits.set(0);
43-
}
40+
@Nullable
41+
private static BitsCronField zeroNanos = null;
4442

45-
private final BitSet bits;
4643

44+
// we store at most 60 bits, for seconds and minutes, so a 64-bit long suffices
45+
private long bits;
4746

4847

4948
private BitsCronField(Type type) {
5049
super(type);
51-
this.bits = new BitSet((int) type.range().getMaximum());
5250
}
5351

5452
/**
5553
* Return a {@code BitsCronField} enabled for 0 nano seconds.
5654
*/
5755
public static BitsCronField zeroNanos() {
58-
return BitsCronField.ZERO_NANOS;
56+
if (zeroNanos == null) {
57+
BitsCronField field = new BitsCronField(Type.NANO);
58+
field.setBit(0);
59+
zeroNanos = field;
60+
}
61+
return zeroNanos;
5962
}
6063

6164
/**
@@ -98,11 +101,10 @@ public static BitsCronField parseMonth(String value) {
98101
*/
99102
public static BitsCronField parseDaysOfWeek(String value) {
100103
BitsCronField result = parseDate(value, Type.DAY_OF_WEEK);
101-
BitSet bits = result.bits;
102-
if (bits.get(0)) {
104+
if (result.getBit(0)) {
103105
// cron supports 0 for Sunday; we use 7 like java.time
104-
bits.set(7);
105-
bits.clear(0);
106+
result.setBit(7);
107+
result.clearBit(0);
106108
}
107109
return result;
108110
}
@@ -173,10 +175,10 @@ private static ValueRange parseRange(String value, Type type) {
173175
@Override
174176
public <T extends Temporal & Comparable<? super T>> T nextOrSame(T temporal) {
175177
int current = type().get(temporal);
176-
int next = this.bits.nextSetBit(current);
178+
int next = nextSetBit(current);
177179
if (next == -1) {
178180
temporal = type().rollForward(temporal);
179-
next = this.bits.nextSetBit(0);
181+
next = nextSetBit(0);
180182
}
181183
if (next == current) {
182184
return temporal;
@@ -195,23 +197,54 @@ public <T extends Temporal & Comparable<? super T>> T nextOrSame(T temporal) {
195197
}
196198
}
197199

198-
BitSet bits() {
199-
return this.bits;
200+
boolean getBit(int index) {
201+
return (this.bits & (1L << index)) != 0;
202+
}
203+
204+
private int nextSetBit(int fromIndex) {
205+
long result = this.bits & (MASK << fromIndex);
206+
if (result != 0) {
207+
return Long.numberOfTrailingZeros(result);
208+
}
209+
else {
210+
return -1;
211+
}
212+
200213
}
201214

202215
private void setBits(ValueRange range) {
203-
this.bits.set((int) range.getMinimum(), (int) range.getMaximum() + 1);
216+
if (range.getMinimum() == range.getMaximum()) {
217+
setBit((int) range.getMinimum());
218+
}
219+
else {
220+
long minMask = MASK << range.getMinimum();
221+
long maxMask = MASK >>> - (range.getMaximum() + 1);
222+
this.bits |= (minMask & maxMask);
223+
}
204224
}
205225

206226
private void setBits(ValueRange range, int delta) {
207-
for (int i = (int) range.getMinimum(); i <= range.getMaximum(); i += delta) {
208-
this.bits.set(i);
227+
if (delta == 1) {
228+
setBits(range);
209229
}
230+
else {
231+
for (int i = (int) range.getMinimum(); i <= range.getMaximum(); i += delta) {
232+
setBit(i);
233+
}
234+
}
235+
}
236+
237+
private void setBit(int index) {
238+
this.bits |= (1L << index);
239+
}
240+
241+
private void clearBit(int index) {
242+
this.bits &= ~(1L << index);
210243
}
211244

212245
@Override
213246
public int hashCode() {
214-
return this.bits.hashCode();
247+
return Long.hashCode(this.bits);
215248
}
216249

217250
@Override
@@ -223,13 +256,25 @@ public boolean equals(Object o) {
223256
return false;
224257
}
225258
BitsCronField other = (BitsCronField) o;
226-
return type() == other.type() &&
227-
this.bits.equals(other.bits);
259+
return type() == other.type() && this.bits == other.bits;
228260
}
229261

230262
@Override
231263
public String toString() {
232-
return type() + " " + this.bits;
264+
StringBuilder builder = new StringBuilder(type().toString());
265+
builder.append(" {");
266+
int i = nextSetBit(0);
267+
if (i != -1) {
268+
builder.append(i);
269+
i = nextSetBit(i+1);
270+
while (i != -1) {
271+
builder.append(", ");
272+
builder.append(i);
273+
i = nextSetBit(i+1);
274+
}
275+
}
276+
builder.append('}');
277+
return builder.toString();
233278
}
234279

235280
}

‎spring-context/src/main/java/org/springframework/scheduling/support/CronField.java

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -252,6 +252,11 @@ public <T extends Temporal> T reset(T temporal) {
252252
}
253253
return temporal;
254254
}
255+
256+
@Override
257+
public String toString() {
258+
return this.field.toString();
259+
}
255260
}
256261

257262
}

‎spring-context/src/test/java/org/springframework/scheduling/support/BitSetAssert.java

Lines changed: 0 additions & 81 deletions
This file was deleted.

‎spring-context/src/test/java/org/springframework/scheduling/support/BitsCronFieldTests.java

Lines changed: 78 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -16,10 +16,13 @@
1616

1717
package org.springframework.scheduling.support;
1818

19+
import java.util.Arrays;
20+
21+
import org.assertj.core.api.Condition;
1922
import org.junit.jupiter.api.Test;
2023

24+
import static org.assertj.core.api.Assertions.assertThat;
2125
import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException;
22-
import static org.springframework.scheduling.support.BitSetAssert.assertThat;
2326

2427
/**
2528
* @author Arjen Poutsma
@@ -28,12 +31,12 @@ public class BitsCronFieldTests {
2831

2932
@Test
3033
void parse() {
31-
assertThat(BitsCronField.parseSeconds("42").bits()).hasUnsetRange(0, 41).hasSet(42).hasUnsetRange(43, 59);
32-
assertThat(BitsCronField.parseMinutes("1,2,5,9").bits()).hasUnset(0).hasSet(1, 2).hasUnset(3,4).hasSet(5).hasUnsetRange(6,8).hasSet(9).hasUnsetRange(10,59);
33-
assertThat(BitsCronField.parseSeconds("0-4,8-12").bits()).hasSetRange(0, 4).hasUnsetRange(5,7).hasSetRange(8, 12).hasUnsetRange(13,59);
34-
assertThat(BitsCronField.parseHours("0-23/2").bits()).hasSet(0, 2, 4, 6, 8, 10, 12, 14, 16, 18, 20, 22).hasUnset(1,3,5,7,9,11,13,15,17,19,21,23);
35-
assertThat(BitsCronField.parseDaysOfWeek("0").bits()).hasUnsetRange(0, 6).hasSet(7, 7);
36-
assertThat(BitsCronField.parseSeconds("57/2").bits()).hasUnsetRange(0, 56).hasSet(57).hasUnset(58).hasSet(59);
34+
assertThat(BitsCronField.parseSeconds("42")).has(clearRange(0, 41)).has(set(42)).has(clearRange(43, 59));
35+
assertThat(BitsCronField.parseMinutes("1,2,5,9")).has(clear(0)).has(set(1, 2)).has(clearRange(3,4)).has(set(5)).has(clearRange(6,8)).has(set(9)).has(clearRange(10,59));
36+
assertThat(BitsCronField.parseSeconds("0-4,8-12")).has(setRange(0, 4)).has(clearRange(5,7)).has(setRange(8, 12)).has(clearRange(13,59));
37+
assertThat(BitsCronField.parseHours("0-23/2")).has(set(0, 2, 4, 6, 8, 10, 12, 14, 16, 18, 20, 22)).has(clear(1,3,5,7,9,11,13,15,17,19,21,23));
38+
assertThat(BitsCronField.parseDaysOfWeek("0")).has(clearRange(0, 6)).has(set(7, 7));
39+
assertThat(BitsCronField.parseSeconds("57/2")).has(clearRange(0, 56)).has(set(57)).has(clear(58)).has(set(59));
3740
}
3841

3942
@Test
@@ -55,22 +58,78 @@ void invalidRange() {
5558

5659
@Test
5760
void parseWildcards() {
58-
assertThat(BitsCronField.parseSeconds("*").bits()).hasSetRange(0, 60);
59-
assertThat(BitsCronField.parseMinutes("*").bits()).hasSetRange(0, 60);
60-
assertThat(BitsCronField.parseHours("*").bits()).hasSetRange(0, 23);
61-
assertThat(BitsCronField.parseDaysOfMonth("*").bits()).hasUnset(0).hasSetRange(1, 31);
62-
assertThat(BitsCronField.parseDaysOfMonth("?").bits()).hasUnset(0).hasSetRange(1, 31);
63-
assertThat(BitsCronField.parseMonth("*").bits()).hasUnset(0).hasSetRange(1, 12);
64-
assertThat(BitsCronField.parseDaysOfWeek("*").bits()).hasUnset(0).hasSetRange(1, 7);
65-
assertThat(BitsCronField.parseDaysOfWeek("?").bits()).hasUnset(0).hasSetRange(1, 7);
61+
assertThat(BitsCronField.parseSeconds("*")).has(setRange(0, 60));
62+
assertThat(BitsCronField.parseMinutes("*")).has(setRange(0, 60));
63+
assertThat(BitsCronField.parseHours("*")).has(setRange(0, 23));
64+
assertThat(BitsCronField.parseDaysOfMonth("*")).has(clear(0)).has(setRange(1, 31));
65+
assertThat(BitsCronField.parseDaysOfMonth("?")).has(clear(0)).has(setRange(1, 31));
66+
assertThat(BitsCronField.parseMonth("*")).has(clear(0)).has(setRange(1, 12));
67+
assertThat(BitsCronField.parseDaysOfWeek("*")).has(clear(0)).has(setRange(1, 7));
68+
assertThat(BitsCronField.parseDaysOfWeek("?")).has(clear(0)).has(setRange(1, 7));
6669
}
6770

6871
@Test
6972
void names() {
70-
assertThat(((BitsCronField)CronField.parseMonth("JAN,FEB,MAR,APR,MAY,JUN,JUL,AUG,SEP,OCT,NOV,DEC")).bits())
71-
.hasUnset(0).hasSetRange(1, 12);
72-
assertThat(((BitsCronField)CronField.parseDaysOfWeek("SUN,MON,TUE,WED,THU,FRI,SAT")).bits())
73-
.hasUnset(0).hasSetRange(1, 7);
73+
assertThat(((BitsCronField)CronField.parseMonth("JAN,FEB,MAR,APR,MAY,JUN,JUL,AUG,SEP,OCT,NOV,DEC")))
74+
.has(clear(0)).has(setRange(1, 12));
75+
assertThat(((BitsCronField)CronField.parseDaysOfWeek("SUN,MON,TUE,WED,THU,FRI,SAT")))
76+
.has(clear(0)).has(setRange(1, 7));
77+
}
78+
79+
private static Condition<BitsCronField> set(int... indices) {
80+
return new Condition<BitsCronField>(String.format("set bits %s", Arrays.toString(indices))) {
81+
@Override
82+
public boolean matches(BitsCronField value) {
83+
for (int index : indices) {
84+
if (!value.getBit(index)) {
85+
return false;
86+
}
87+
}
88+
return true;
89+
}
90+
};
91+
}
92+
93+
private static Condition<BitsCronField> setRange(int min, int max) {
94+
return new Condition<BitsCronField>(String.format("set range %d-%d", min, max)) {
95+
@Override
96+
public boolean matches(BitsCronField value) {
97+
for (int i = min; i < max; i++) {
98+
if (!value.getBit(i)) {
99+
return false;
100+
}
101+
}
102+
return true;
103+
}
104+
};
105+
}
106+
107+
private static Condition<BitsCronField> clear(int... indices) {
108+
return new Condition<BitsCronField>(String.format("clear bits %s", Arrays.toString(indices))) {
109+
@Override
110+
public boolean matches(BitsCronField value) {
111+
for (int index : indices) {
112+
if (value.getBit(index)) {
113+
return false;
114+
}
115+
}
116+
return true;
117+
}
118+
};
119+
}
120+
121+
private static Condition<BitsCronField> clearRange(int min, int max) {
122+
return new Condition<BitsCronField>(String.format("clear range %d-%d", min, max)) {
123+
@Override
124+
public boolean matches(BitsCronField value) {
125+
for (int i = min; i < max; i++) {
126+
if (value.getBit(i)) {
127+
return false;
128+
}
129+
}
130+
return true;
131+
}
132+
};
74133
}
75134

76135
}

0 commit comments

Comments
 (0)