Skip to content

Commit de87119

Browse files
justinhorvitzcopybara-github
authored andcommitted
Use specialized fastutil maps in serialization to avoid boxing of primitive ints.
PiperOrigin-RevId: 587006554 Change-Id: I006548544656f40a976a038cddd259d57ec15611
1 parent 41ca39a commit de87119

File tree

3 files changed

+47
-41
lines changed

3 files changed

+47
-41
lines changed

src/main/java/com/google/devtools/build/lib/skyframe/serialization/BUILD

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ java_library(
3939
"//src/main/java/com/google/devtools/build/lib/unsafe:string",
4040
"//src/main/java/com/google/devtools/build/lib/unsafe:unsafe-provider",
4141
"//third_party:error_prone_annotations",
42+
"//third_party:fastutil",
4243
"//third_party:flogger",
4344
"//third_party:guava",
4445
"//third_party:jsr305",

src/main/java/com/google/devtools/build/lib/skyframe/serialization/Memoizer.java

Lines changed: 44 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -19,11 +19,11 @@
1919
import com.google.devtools.build.lib.skyframe.serialization.ObjectCodec.MemoizationStrategy;
2020
import com.google.protobuf.CodedInputStream;
2121
import com.google.protobuf.CodedOutputStream;
22+
import it.unimi.dsi.fastutil.ints.Int2ObjectOpenHashMap;
23+
import it.unimi.dsi.fastutil.objects.Reference2IntOpenHashMap;
2224
import java.io.IOException;
2325
import java.util.ArrayDeque;
2426
import java.util.Deque;
25-
import java.util.HashMap;
26-
import java.util.IdentityHashMap;
2727
import javax.annotation.Nullable;
2828

2929
/**
@@ -157,9 +157,8 @@ <T> void serialize(
157157
}
158158
}
159159

160-
@Nullable
161-
Integer getMemoizedIndex(Object obj) {
162-
return memo.lookupNullable(obj);
160+
int getMemoizedIndex(Object obj) {
161+
return memo.lookup(obj);
163162
}
164163

165164
// Corresponds to MemoContent in the abstract grammar.
@@ -170,52 +169,55 @@ private <T> void serializeMemoContent(
170169
CodedOutputStream codedOut,
171170
MemoizationStrategy strategy)
172171
throws SerializationException, IOException {
173-
switch(strategy) {
174-
case MEMOIZE_BEFORE: {
175-
int id = memo.memoize(obj);
176-
codedOut.writeInt32NoTag(id);
172+
switch (strategy) {
173+
case MEMOIZE_BEFORE:
174+
{
175+
int id = memo.memoize(obj);
176+
codedOut.writeInt32NoTag(id);
177177
codec.serialize(context, obj, codedOut);
178-
break;
179-
}
180-
case MEMOIZE_AFTER: {
178+
break;
179+
}
180+
case MEMOIZE_AFTER:
181+
{
181182
codec.serialize(context, obj, codedOut);
182-
// If serializing the children caused the parent object itself to be serialized due to a
183-
// cycle, then there's now a memo entry for the parent. Don't overwrite it with a new id.
184-
Integer cylicallyCreatedId = memo.lookupNullable(obj);
185-
int id = (cylicallyCreatedId != null) ? cylicallyCreatedId : memo.memoize(obj);
186-
codedOut.writeInt32NoTag(id);
187-
break;
188-
}
183+
// If serializing the children caused the parent object itself to be serialized due to a
184+
// cycle, then there's now a memo entry for the parent. Don't overwrite it with a new
185+
// id.
186+
int cylicallyCreatedId = memo.lookup(obj);
187+
int id = (cylicallyCreatedId != -1) ? cylicallyCreatedId : memo.memoize(obj);
188+
codedOut.writeInt32NoTag(id);
189+
break;
190+
}
189191
default:
190192
throw new AssertionError("Unreachable (strategy=" + strategy + ")");
191193
}
192194
}
193195

194196
private static class SerializingMemoTable {
195-
private final IdentityHashMap<Object, Integer> table = new IdentityHashMap<>();
197+
private final Reference2IntOpenHashMap<Object> table = new Reference2IntOpenHashMap<>();
198+
199+
SerializingMemoTable() {
200+
table.defaultReturnValue(-1);
201+
}
196202

197203
/**
198204
* Adds a new value to the memo table and returns its id. The value must not already be
199205
* present.
200206
*/
201207
private int memoize(Object value) {
202208
Preconditions.checkArgument(
203-
!table.containsKey(value),
204-
"Tried to memoize object '%s' multiple times", value);
209+
!table.containsKey(value), "Tried to memoize object '%s' multiple times", value);
205210
// Ids count sequentially from 0.
206211
int newId = table.size();
207212
table.put(value, newId);
208213
return newId;
209214
}
210215

211216
/**
212-
* If the value is already memoized, return its on-the-wire id; otherwise return null. Opaque.
213-
*
214-
* <p>Beware accidental unboxing of a null result.
217+
* If the value is already memoized, return its on-the-wire id; otherwise returns {@code -1}.
215218
*/
216-
@Nullable
217-
private Integer lookupNullable(Object value) {
218-
return table.get(value);
219+
private int lookup(Object value) {
220+
return table.getInt(value);
219221
}
220222
}
221223
}
@@ -225,7 +227,7 @@ private Integer lookupNullable(Object value) {
225227
*/
226228
static class Deserializer {
227229
private final DeserializingMemoTable memo = new DeserializingMemoTable();
228-
@Nullable private Integer tagForMemoizedBefore = null;
230+
private int tagForMemoizedBefore = -1;
229231
private final Deque<Object> memoizedBeforeStackForSanityChecking = new ArrayDeque<>();
230232

231233
/**
@@ -241,7 +243,7 @@ <T> T deserialize(
241243
CodedInputStream codedIn)
242244
throws SerializationException, IOException {
243245
Preconditions.checkState(
244-
tagForMemoizedBefore == null,
246+
tagForMemoizedBefore == -1,
245247
"non-null memoized-before tag %s (%s)",
246248
tagForMemoizedBefore,
247249
codec);
@@ -302,10 +304,10 @@ private static <T> T castedDeserialize(
302304
}
303305

304306
<T> void registerInitialValue(T initialValue) {
305-
int tag =
306-
Preconditions.checkNotNull(
307-
tagForMemoizedBefore, " Not called with memoize before: %s", initialValue);
308-
tagForMemoizedBefore = null;
307+
Preconditions.checkState(
308+
tagForMemoizedBefore != -1, "Not called with memoize before: %s", initialValue);
309+
int tag = tagForMemoizedBefore;
310+
tagForMemoizedBefore = -1;
309311
memo.memoize(tag, initialValue);
310312
memoizedBeforeStackForSanityChecking.addLast(initialValue);
311313
}
@@ -348,18 +350,21 @@ private <T> T deserializeMemoAfterContent(
348350

349351
private static class DeserializingMemoTable {
350352

351-
private HashMap<Integer, Object> table = new HashMap<>();
353+
private final Int2ObjectOpenHashMap<Object> table = new Int2ObjectOpenHashMap<>();
352354

353355
/**
354356
* Adds a new id -> object maplet to the memo table. The value must not already be present.
355357
*/
356358
private void memoize(int id, Object value) {
357359
Preconditions.checkNotNull(value);
358360
Object prev = table.put(id, value);
359-
Preconditions.checkArgument(
360-
prev == null,
361-
"Tried to memoize id %s to object '%s', when it is already memoized to object '%s'",
362-
id, value, prev);
361+
if (prev != null) { // Avoid boxing int with checkArgument.
362+
throw new IllegalArgumentException(
363+
String.format(
364+
"Tried to memoize id %s to object '%s', when it is already memoized to object"
365+
+ " '%s'",
366+
id, value, prev));
367+
}
363368
}
364369

365370
/** If the id has been memoized, returns its corresponding object. Otherwise returns null. */

src/main/java/com/google/devtools/build/lib/skyframe/serialization/SerializationContext.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -278,8 +278,8 @@ private ObjectCodecRegistry.CodecDescriptor recordAndGetDescriptorIfNotConstantM
278278
return null;
279279
}
280280
if (serializer != null) {
281-
Integer memoizedIndex = serializer.getMemoizedIndex(object);
282-
if (memoizedIndex != null) {
281+
int memoizedIndex = serializer.getMemoizedIndex(object);
282+
if (memoizedIndex != -1) {
283283
// Subtract 1 so it will be negative and not collide with null.
284284
codedOut.writeSInt32NoTag(-memoizedIndex - 1);
285285
return null;

0 commit comments

Comments
 (0)