Skip to content

Commit b5f2b34

Browse files
brandjoncopybara-github
authored andcommitted
Rename StarlarkClassDescriptor -> ClassDescriptor
This is more symmetrical to `MethodDescriptor` and `ParamDescriptor`. The name "ClassDescriptor" is occasionally used in various contexts in the Java ecosystem, usually in relation to serialization, but I don't anticipate any significant ambiguity. Work toward bazelbuild#28325. PiperOrigin-RevId: 866096241 Change-Id: Iefea0360229c1cb8c21f10e9a0a41c3c72d2ff8f
1 parent 5a99b45 commit b5f2b34

File tree

4 files changed

+34
-43
lines changed

4 files changed

+34
-43
lines changed

src/main/java/com/google/devtools/build/lib/actions/PathMapper.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ default StarlarkSemantics storeIn(StarlarkSemantics semantics) {
7777
// comparisons of equal but not reference equal semantics maps, which regresses CPU (~7% on
7878
// a benchmark with ~10 semantics options).
7979
@Override
80-
public StarlarkSemantics getStarlarkClassDescriptorCacheKey() {
80+
public StarlarkSemantics getClassDescriptorCacheKey() {
8181
return semantics;
8282
}
8383
};

src/main/java/net/starlark/java/eval/CallUtils.java

Lines changed: 31 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -29,14 +29,12 @@ final class CallUtils {
2929
private CallUtils() {} // uninstantiable
3030

3131
/**
32-
* Returns the {@link StarlarkClassDescriptor} for the given {@link StarlarkSemantics} and {@link
33-
* Class}.
32+
* Returns the {@link ClassDescriptor} for the given {@link StarlarkSemantics} and {@link Class}.
3433
*
3534
* <p>This method is a hotspot! It's called on every function call and field access. A single
3635
* `bazel build` invocation can make tens or even hundreds of millions of calls to this method.
3736
*/
38-
private static StarlarkClassDescriptor getStarlarkClassDescriptor(
39-
StarlarkSemantics semantics, Class<?> clazz) {
37+
private static ClassDescriptor getClassDescriptor(StarlarkSemantics semantics, Class<?> clazz) {
4038
if (clazz == String.class) {
4139
clazz = StringModule.class;
4240
}
@@ -48,15 +46,14 @@ private static StarlarkClassDescriptor getStarlarkClassDescriptor(
4846
// structure, and the GC churn and method call overhead become meaningful at scale.
4947
//
5048
// We implement each cache ourselves using CHM#get and CHM#putIfAbsent. We don't use
51-
// CHM#computeIfAbsent since it is not reentrant: If #getStarlarkClassDescriptor is called
49+
// CHM#computeIfAbsent since it is not reentrant: If #getClassDescriptor is called
5250
// before Starlark.UNIVERSE is initialized then the computation will re-enter the cache and have
5351
// a cycle; see b/161479826 for history.
5452
// TODO(bazel-team): Maybe the above cycle concern doesn't exist now that CallUtils is private.
55-
ConcurrentHashMap<Class<?>, StarlarkClassDescriptor> starlarkClassDescriptorCache =
56-
starlarkClassDescriptorCachesBySemantics.get(
57-
semantics.getStarlarkClassDescriptorCacheKey());
58-
if (starlarkClassDescriptorCache == null) {
59-
starlarkClassDescriptorCache =
53+
ConcurrentHashMap<Class<?>, ClassDescriptor> classDescriptorCache =
54+
classDescriptorCachesBySemantics.get(semantics.getClassDescriptorCacheKey());
55+
if (classDescriptorCache == null) {
56+
classDescriptorCache =
6057
new ConcurrentHashMap<>(
6158
// In May 2023, typical Bazel usage results in ~150 entries in this cache. Therefore
6259
// we presize the CHM accordingly to reduce the chance two entries use the same hash
@@ -71,24 +68,22 @@ private static StarlarkClassDescriptor getStarlarkClassDescriptor(
7168
// concerns, so we can use a more efficient data structure that doesn't need to
7269
// handle concurrent writes.
7370
/* initialCapacity= */ 1000);
74-
ConcurrentHashMap<Class<?>, StarlarkClassDescriptor> prev =
75-
starlarkClassDescriptorCachesBySemantics.putIfAbsent(
76-
semantics, starlarkClassDescriptorCache);
71+
ConcurrentHashMap<Class<?>, ClassDescriptor> prev =
72+
classDescriptorCachesBySemantics.putIfAbsent(semantics, classDescriptorCache);
7773
if (prev != null) {
78-
starlarkClassDescriptorCache = prev; // first thread wins
74+
classDescriptorCache = prev; // first thread wins
7975
}
8076
}
8177

82-
StarlarkClassDescriptor starlarkClassDescriptor = starlarkClassDescriptorCache.get(clazz);
83-
if (starlarkClassDescriptor == null) {
84-
starlarkClassDescriptor = buildStarlarkClassDescriptor(semantics, clazz);
85-
StarlarkClassDescriptor prev =
86-
starlarkClassDescriptorCache.putIfAbsent(clazz, starlarkClassDescriptor);
78+
ClassDescriptor classDescriptor = classDescriptorCache.get(clazz);
79+
if (classDescriptor == null) {
80+
classDescriptor = buildClassDescriptor(semantics, clazz);
81+
ClassDescriptor prev = classDescriptorCache.putIfAbsent(clazz, classDescriptor);
8782
if (prev != null) {
88-
starlarkClassDescriptor = prev; // first thread wins
83+
classDescriptor = prev; // first thread wins
8984
}
9085
}
91-
return starlarkClassDescriptor;
86+
return classDescriptor;
9287
}
9388

9489
/**
@@ -98,17 +93,17 @@ private static StarlarkClassDescriptor getStarlarkClassDescriptor(
9893
* <p>Generally, but not always (e.g. in the case of compilations of global functions like {@link
9994
* MethodLibrary}), instances of the Java class are valid as Starlark values.
10095
*
101-
* <p>Although a {@code StarlarkClassDescriptor} does not directly embed the {@code
102-
* StarlarkSemantics}, its contents vary based on them. In contrast, {@link MethodDescriptor} and
103-
* {@link ParamDescriptor} do not vary with the semantics.
96+
* <p>Although a {@code ClassDescriptor} does not directly embed the {@code StarlarkSemantics},
97+
* its contents vary based on them. In contrast, {@link MethodDescriptor} and {@link
98+
* ParamDescriptor} do not vary with the semantics.
10499
*/
105100
// TODO(bazel-team): For context on whether descriptors should depend on the StarlarkSemantics,
106101
// see #25743 and the discussion in cl/742265869. The history of this is that eliminating the
107102
// dependence on semantics made it simpler to obtain type information and avoid an overreliance on
108103
// StarlarkSemantics#DEFAULT. But embedding a semantics may make it simpler to give precise static
109104
// type information that takes into account flag-guarding. For the moment it suffices to store a
110105
// semantics in BuiltinFunction.
111-
private static class StarlarkClassDescriptor {
106+
private static class ClassDescriptor {
112107
/**
113108
* The descriptor for the unique {@code @StarlarkMethod}-annotated method on this class that has
114109
* {@link StarlarkMethod#selfCall} set to true (ex: "struct" in Bazel), or null if there is no
@@ -130,16 +125,12 @@ private static class StarlarkClassDescriptor {
130125
ImmutableMap<String, MethodDescriptor> methods;
131126
}
132127

133-
/**
134-
* Two-layer cache of {@link #buildStarlarkClassDescriptor}, managed by {@link
135-
* #getStarlarkClassDescriptor}.
136-
*/
128+
/** Two-layer cache of {@link #buildClassDescriptor}, managed by {@link #getClassDescriptor}. */
137129
private static final ConcurrentHashMap<
138-
StarlarkSemantics, ConcurrentHashMap<Class<?>, StarlarkClassDescriptor>>
139-
starlarkClassDescriptorCachesBySemantics = new ConcurrentHashMap<>();
130+
StarlarkSemantics, ConcurrentHashMap<Class<?>, ClassDescriptor>>
131+
classDescriptorCachesBySemantics = new ConcurrentHashMap<>();
140132

141-
private static StarlarkClassDescriptor buildStarlarkClassDescriptor(
142-
StarlarkSemantics semantics, Class<?> clazz) {
133+
private static ClassDescriptor buildClassDescriptor(StarlarkSemantics semantics, Class<?> clazz) {
143134
MethodDescriptor selfCall = null;
144135
ImmutableMap.Builder<String, MethodDescriptor> methods = ImmutableMap.builder();
145136

@@ -180,10 +171,10 @@ private static StarlarkClassDescriptor buildStarlarkClassDescriptor(
180171
methods.put(callable.name(), descriptor);
181172
}
182173

183-
StarlarkClassDescriptor starlarkClassDescriptor = new StarlarkClassDescriptor();
184-
starlarkClassDescriptor.selfCall = selfCall;
185-
starlarkClassDescriptor.methods = methods.buildOrThrow();
186-
return starlarkClassDescriptor;
174+
ClassDescriptor classDescriptor = new ClassDescriptor();
175+
classDescriptor.selfCall = selfCall;
176+
classDescriptor.methods = methods.buildOrThrow();
177+
return classDescriptor;
187178
}
188179

189180
/**
@@ -192,7 +183,7 @@ private static StarlarkClassDescriptor buildStarlarkClassDescriptor(
192183
*/
193184
static ImmutableMap<String, MethodDescriptor> getAnnotatedMethods(
194185
StarlarkSemantics semantics, Class<?> objClass) {
195-
return getStarlarkClassDescriptor(semantics, objClass).methods;
186+
return getClassDescriptor(semantics, objClass).methods;
196187
}
197188

198189
/**
@@ -203,7 +194,7 @@ static ImmutableMap<String, MethodDescriptor> getAnnotatedMethods(
203194
@Nullable
204195
static MethodDescriptor getSelfCallMethodDescriptor(
205196
StarlarkSemantics semantics, Class<?> objClass) {
206-
return getStarlarkClassDescriptor(semantics, objClass).selfCall;
197+
return getClassDescriptor(semantics, objClass).selfCall;
207198
}
208199

209200
/**
@@ -212,7 +203,7 @@ static MethodDescriptor getSelfCallMethodDescriptor(
212203
*/
213204
@Nullable
214205
static Method getSelfCallMethod(StarlarkSemantics semantics, Class<?> objClass) {
215-
MethodDescriptor descriptor = getStarlarkClassDescriptor(semantics, objClass).selfCall;
206+
MethodDescriptor descriptor = getClassDescriptor(semantics, objClass).selfCall;
216207
if (descriptor == null) {
217208
return null;
218209
}

src/main/java/net/starlark/java/eval/Starlark.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -334,7 +334,7 @@ static StarlarkType getStarlarkType(Object value) {
334334
case StarlarkValue x -> {
335335
@Nullable StarlarkType type = x.getStarlarkType();
336336
if (type == null) {
337-
// TODO: #28325 - For types with StarlarkClassDescriptors, return the type stored in the
337+
// TODO: #28325 - For types with ClassDescriptors, return the type stored in the
338338
// descriptor.
339339
type = Types.ANY;
340340
}

src/main/java/net/starlark/java/eval/StarlarkSemantics.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -211,7 +211,7 @@ final boolean isFeatureEnabledBasedOnTogglingFlags(String enablingFlag, String d
211211
* Returns a possibly different {@link StarlarkSemantics} instance that is equivalent to this one
212212
* for the purpose of caching the methods available on any given Starlark class.
213213
*/
214-
public StarlarkSemantics getStarlarkClassDescriptorCacheKey() {
214+
public StarlarkSemantics getClassDescriptorCacheKey() {
215215
return this;
216216
}
217217

0 commit comments

Comments
 (0)