Skip to content

Commit 0c324b1

Browse files
committed
Removing immutable
Removed immutable which is a bit dangerous and not needed with both freeze and immutableCopy being available Tweaked special constructor used by createEmpty (and previously by immutable). Now takes a size to be a bit more explicitly about what is happening.
1 parent 176846f commit 0c324b1

File tree

1 file changed

+62
-67
lines changed
  • internal-api/src/main/java/datadog/trace/api

1 file changed

+62
-67
lines changed

internal-api/src/main/java/datadog/trace/api/TagMap.java

Lines changed: 62 additions & 67 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@
3434
* Entries must be replaced by re-setting / re-putting the key which will create a new Entry object.
3535
*
3636
* <p>This map also lacks features designed for handling large long lived mutable maps...
37+
*
3738
* <ul>
3839
* <li>bucket array expansion
3940
* <li>adaptive collision
@@ -67,7 +68,7 @@ private static final TagMap createEmpty() {
6768
// Using special constructor that creates a frozen view of an existing array
6869
// Bucket calculation requires that array length is a power of 2
6970
// e.g. size 0 will not work, it results in ArrayIndexOutOfBoundsException, but size 1 does
70-
return new TagMap(new Object[1]);
71+
return new TagMap(new Object[1], 0);
7172
}
7273

7374
/** Creates a new TagMap.Builder */
@@ -108,9 +109,9 @@ public TagMap() {
108109
}
109110

110111
/** Used for inexpensive immutable */
111-
private TagMap(Object[] buckets) {
112+
private TagMap(Object[] buckets, int size) {
112113
this.buckets = buckets;
113-
this.size = 0;
114+
this.size = size;
114115
this.frozen = true;
115116
}
116117

@@ -359,7 +360,7 @@ public final void putAll(Map<? extends String, ? extends Object> map) {
359360
this.putAll((TagMap) map);
360361
} else {
361362
for (Map.Entry<? extends String, ?> entry : map.entrySet()) {
362-
// use set which returns a prior Entry rather put which may box a prior primitive value
363+
// use set which returns a prior Entry rather put which may box a prior primitive value
363364
this.set(entry.getKey(), entry.getValue());
364365
}
365366
}
@@ -373,29 +374,30 @@ public final void putAll(Map<? extends String, ? extends Object> map) {
373374
* source TagMap
374375
*/
375376
public final void putAll(TagMap that) {
376-
this.checkWriteAccess();
377-
378-
if ( this.size == 0 ) {
379-
this.putAllIntoEmptyMap(that);
380-
} else {
381-
this.putAllMerge(that);
382-
}
383-
}
384-
377+
this.checkWriteAccess();
378+
379+
if (this.size == 0) {
380+
this.putAllIntoEmptyMap(that);
381+
} else {
382+
this.putAllMerge(that);
383+
}
384+
}
385+
385386
private final void putAllMerge(TagMap that) {
386387
Object[] thisBuckets = this.buckets;
387388
Object[] thatBuckets = that.buckets;
388389

389390
// Since TagMap-s don't support expansion, buckets are perfectly aligned
390-
// Check against both thisBuckets.length && thatBuckets.length is to help the JIT do bound check elimination
391+
// Check against both thisBuckets.length && thatBuckets.length is to help the JIT do bound check
392+
// elimination
391393
for (int i = 0; i < thisBuckets.length && i < thatBuckets.length; ++i) {
392-
Object thatBucket = thatBuckets[i];
393-
394-
// if nothing incoming, nothing to do
395-
if ( thatBucket == null ) continue;
394+
Object thatBucket = thatBuckets[i];
395+
396+
// if nothing incoming, nothing to do
397+
if (thatBucket == null) continue;
396398

397399
Object thisBucket = thisBuckets[i];
398-
if (thisBucket == null) {
400+
if (thisBucket == null) {
399401
// This bucket is null, easy case
400402
// Either copy over the sole entry or clone the BucketGroup chain
401403

@@ -444,19 +446,19 @@ private final void putAllMerge(TagMap that) {
444446
// there's already an entry w/ the same tag from the incoming TagMap
445447
// incoming entry clobbers the existing try, so we're done
446448
thisBuckets[i] = thisNewGroup;
447-
449+
448450
// overlapping group - subtract one for clobbered existing entry
449-
this.size += thisNewGroupSize - 1;
451+
this.size += thisNewGroupSize - 1;
450452
} else if (thisNewGroup.insertInChain(thisHash, thisEntry)) {
451453
// able to add thisEntry into the existing groups
452454
thisBuckets[i] = thisNewGroup;
453-
455+
454456
// non overlapping group - existing entry already accounted for in this.size
455-
this.size += thisNewGroupSize;
457+
this.size += thisNewGroupSize;
456458
} else {
457459
// unable to add into the existing groups
458460
thisBuckets[i] = new BucketGroup(thisHash, thisEntry, thisNewGroup);
459-
461+
460462
// non overlapping group - existing entry already accounted for in this.size
461463
this.size += thisNewGroupSize;
462464
}
@@ -494,28 +496,29 @@ private final void putAllMerge(TagMap that) {
494496
}
495497
}
496498
}
497-
499+
498500
/*
499501
* Optimized special case method for the common case of putAll into an empty TagMap used by copy, etc
500502
*/
501503
private final void putAllIntoEmptyMap(TagMap that) {
502504
Object[] thisBuckets = this.buckets;
503505
Object[] thatBuckets = that.buckets;
504506

505-
// Check against both thisBuckets.length && thatBuckets.length is to help the JIT do bound check elimination
507+
// Check against both thisBuckets.length && thatBuckets.length is to help the JIT do bound check
508+
// elimination
506509
for (int i = 0; i < thisBuckets.length && i < thatBuckets.length; ++i) {
507510
Object thatBucket = thatBuckets[i];
508511

509512
// faster to explicitly null check first, then do instanceof
510-
if ( thatBucket == null ) {
511-
// do nothing
512-
} else if ( thatBucket instanceof BucketGroup ) {
513-
// if it is a BucketGroup, then need to clone
514-
BucketGroup thatGroup = (BucketGroup)thatBucket;
515-
516-
thisBuckets[i] = thatGroup.cloneChain();
517-
} else { // if ( thatBucket instanceof Entry )
518-
thisBuckets[i] = thatBucket;
513+
if (thatBucket == null) {
514+
// do nothing
515+
} else if (thatBucket instanceof BucketGroup) {
516+
// if it is a BucketGroup, then need to clone
517+
BucketGroup thatGroup = (BucketGroup) thatBucket;
518+
519+
thisBuckets[i] = thatGroup.cloneChain();
520+
} else { // if ( thatBucket instanceof Entry )
521+
thisBuckets[i] = thatBucket;
519522
}
520523
}
521524
this.size = that.size;
@@ -628,11 +631,6 @@ public final TagMap immutableCopy() {
628631
}
629632
}
630633

631-
public final TagMap immutable() {
632-
// specialized constructor, freezes map immediately
633-
return new TagMap(this.buckets);
634-
}
635-
636634
/**
637635
* Provides an Iterator over the Entry-s of the TagMap Equivalent to <code>entrySet().iterator()
638636
* </code>, but with less allocation
@@ -742,50 +740,47 @@ public final void checkWriteAccess() {
742740
}
743741

744742
final void checkIntegrity() {
745-
// Decided to use if ( cond ) throw new IllegalStateException rather than assert
746-
// That was done to avoid the extra static initialization needed for an assertion
747-
// While that's probably an unnecessary optimization, this method is only called in tests
748-
743+
// Decided to use if ( cond ) throw new IllegalStateException rather than assert
744+
// That was done to avoid the extra static initialization needed for an assertion
745+
// While that's probably an unnecessary optimization, this method is only called in tests
746+
749747
Object[] thisBuckets = this.buckets;
750-
751-
for ( int i = 0; i < thisBuckets.length; ++i ) {
748+
749+
for (int i = 0; i < thisBuckets.length; ++i) {
752750
Object thisBucket = thisBuckets[i];
753-
754-
if ( thisBucket instanceof Entry ) {
755-
Entry thisEntry = (Entry)thisBucket;
751+
752+
if (thisBucket instanceof Entry) {
753+
Entry thisEntry = (Entry) thisBucket;
756754
int thisHash = thisEntry.hash();
757-
755+
758756
int expectedBucket = thisHash & (thisBuckets.length - 1);
759-
if ( expectedBucket != i ) {
757+
if (expectedBucket != i) {
760758
throw new IllegalStateException("incorrect bucket");
761759
}
762-
} else if ( thisBucket instanceof BucketGroup ) {
763-
BucketGroup thisGroup = (BucketGroup)thisBucket;
764-
765-
for ( BucketGroup curGroup = thisGroup;
766-
curGroup != null;
767-
curGroup = curGroup.prev )
768-
{
769-
for ( int j = 0; j < BucketGroup.LEN; ++j ) {
760+
} else if (thisBucket instanceof BucketGroup) {
761+
BucketGroup thisGroup = (BucketGroup) thisBucket;
762+
763+
for (BucketGroup curGroup = thisGroup; curGroup != null; curGroup = curGroup.prev) {
764+
for (int j = 0; j < BucketGroup.LEN; ++j) {
770765
Entry thisEntry = curGroup._entryAt(i);
771-
if ( thisEntry == null ) continue;
772-
766+
if (thisEntry == null) continue;
767+
773768
int thisHash = thisEntry.hash();
774769
assert curGroup._hashAt(i) == thisHash;
775-
770+
776771
int expectedBucket = thisHash & (thisBuckets.length - 1);
777-
if ( expectedBucket != i ) {
772+
if (expectedBucket != i) {
778773
throw new IllegalStateException("incorrect bucket");
779774
}
780775
}
781776
}
782777
}
783778
}
784-
785-
if ( this.size != this.computeSize() ) {
779+
780+
if (this.size != this.computeSize()) {
786781
throw new IllegalStateException("incorrect size");
787782
}
788-
if ( this.isEmpty() != this.checkIfEmpty() ) {
783+
if (this.isEmpty() != this.checkIfEmpty()) {
789784
throw new IllegalStateException("incorrect empty status");
790785
}
791786
}
@@ -806,7 +801,7 @@ final int computeSize() {
806801
}
807802
return size;
808803
}
809-
804+
810805
final boolean checkIfEmpty() {
811806
Object[] thisBuckets = this.buckets;
812807

0 commit comments

Comments
 (0)