spmallette commented on code in PR #3483:
URL: https://github.com/apache/tinkerpop/pull/3483#discussion_r3546304062


##########
tinkergraph-gremlin/src/main/java/org/apache/tinkerpop/gremlin/tinkergraph/structure/TinkerVertex.java:
##########
@@ -54,31 +57,150 @@ public class TinkerVertex extends TinkerElement implements 
Vertex {
     private boolean allowNullPropertyValues;
     private final boolean isTxMode;
 
+    /**
+     * Multi-label storage for this vertex. Starts as an immutable set 
(lightweight) and is
+     * upgraded to a mutable ConcurrentHashMap-backed set on first label 
mutation.
+     */
+    protected Set<String> vertexLabels;
+
     protected TinkerVertex(final Object id, final String label, final 
AbstractTinkerGraph graph) {
-        super(id, label);
-        this.graph = graph;
-        this.isTxMode = graph instanceof TinkerTransactionGraph;
-        this.allowNullPropertyValues = 
graph.features().vertex().supportsNullPropertyValues();
+        this(id, Collections.singleton(label != null ? label : 
Vertex.DEFAULT_LABEL), graph, -1);
     }
 
     protected TinkerVertex(final Object id, final String label, final 
AbstractTinkerGraph graph, final long currentVersion) {
-        super(id, label, currentVersion);
+        this(id, Collections.singleton(label != null ? label : 
Vertex.DEFAULT_LABEL), graph, currentVersion);
+    }
+
+    /**
+     * Constructs a TinkerVertex with multiple labels.
+     */
+    protected TinkerVertex(final Object id, final Set<String> labels, final 
AbstractTinkerGraph graph) {
+        this(id, labels, graph, -1);
+    }
+
+    /**
+     * Canonical constructor. Constructs a TinkerVertex with multiple labels 
and a specific version (for transactional graphs).
+     * Uses a single-switch pattern to handle default label injection per the 
configured cardinality.
+     */
+    protected TinkerVertex(final Object id, final Set<String> labels, final 
AbstractTinkerGraph graph, final long currentVersion) {
+        super(id, null, currentVersion);  // label field set below
         this.graph = graph;
         this.isTxMode = graph instanceof TinkerTransactionGraph;
         this.allowNullPropertyValues = 
graph.features().vertex().supportsNullPropertyValues();
+
+        // Build the initial label set. Use lightweight immutable sets when 
possible.
+        final Set<String> initial = new LinkedHashSet<>();
+        switch (graph.vertexLabelCardinality) {
+            case ONE:
+                // Exactly 1 label: use provided or default
+                if (labels != null && !labels.isEmpty())
+                    initial.addAll(labels);
+                else
+                    initial.add(graph.defaultVertexLabel);
+                break;
+            case ONE_OR_MORE:
+                // Default label always present alongside any user labels
+                initial.add(graph.defaultVertexLabel);
+                if (labels != null) initial.addAll(labels);
+                break;
+            case ZERO_OR_MORE:
+                if (labels != null) initial.addAll(labels);
+                break;
+        }
+
+        // Validate the resulting set conforms to cardinality
+        
LabelCardinalityValidator.validateCreation(graph.vertexLabelCardinality, 
initial);
+
+        // Store as immutable — will be upgraded on first mutation via 
ensureMutableLabels()
+        this.vertexLabels = initial.size() <= 1
+                ? (initial.isEmpty() ? Collections.emptySet() : 
Collections.singleton(initial.iterator().next()))
+                : Collections.unmodifiableSet(initial);
+
+        this.label = this.vertexLabels.isEmpty() ? "" : 
this.vertexLabels.iterator().next();
+    }
+
+    /**
+     * Upgrades the label set to a mutable ConcurrentHashMap-backed set on 
first mutation.
+     * This is a one-time cost per vertex that actually needs label mutation.
+     */
+    private void ensureMutableLabels() {
+        if (!(this.vertexLabels instanceof ConcurrentHashMap.KeySetView)) {
+            final Set<String> mutable = ConcurrentHashMap.newKeySet();
+            mutable.addAll(this.vertexLabels);
+            this.vertexLabels = mutable;
+        }
+    }
+
+    @Override
+    public Set<String> labels() {
+        if (this.vertexLabels.isEmpty()) {
+            return Collections.emptySet();
+        }
+        // If already immutable (pre-mutation), return directly. If mutable 
(post-mutation), wrap.
+        if (this.vertexLabels instanceof ConcurrentHashMap.KeySetView) {
+            return Collections.unmodifiableSet(this.vertexLabels);
+        }
+        return this.vertexLabels; // already immutable from construction
+    }
+
+    @Override
+    @Deprecated
+    public String label() {
+        if (this.vertexLabels.isEmpty()) {
+            return "";
+        }
+        return this.vertexLabels.iterator().next();
+    }
+
+    @Override
+    public void addLabel(final String label, final String... labels) {
+        ElementHelper.validateLabel(label);
+        for (final String l : labels) {
+            ElementHelper.validateLabel(l);
+        }
+        
LabelCardinalityValidator.validateAdd(this.graph.vertexLabelCardinality, 
this.vertexLabels, label, labels);
+        graph.touch(this);
+        ensureMutableLabels();
+        this.vertexLabels.add(label);
+        Collections.addAll(this.vertexLabels, labels);
+        this.label = this.vertexLabels.iterator().next();
+        this.graph.updateVertexLabelIndex(this);

Review Comment:
   we never update the `vertexLabelCounts` with this or with `dropLabels` 
below. 



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to