Skip to content

Commit e810576

Browse files
frsyukifrsyuki
committed
Make copy and reference clear at ValueFactory
ValueFactory .newBinary, .newString, .newArray and .newMap take a mutable argument. Some of them used them as a reference and some of them copied them. This pull-request makes it clear that they copy the argument by default and try to omit the copy if an extra argument is set to true. This change also includes: * newMap(Value... kvs) method which was previously newMap(Value[] kvs) * MessageUnpacker.unpackValue removes a duplicated copy of new byte[] (string and binary) or Value[] (array and map).
1 parent bdb2c2a commit e810576

File tree

2 files changed

+81
-11
lines changed

2 files changed

+81
-11
lines changed

‎msgpack-core/src/main/java/org/msgpack/core/MessageUnpacker.java‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -494,19 +494,19 @@ public ImmutableValue unpackValue()
494494
return ValueFactory.newFloat(unpackDouble());
495495
case STRING: {
496496
int length = unpackRawStringHeader();
497-
return ValueFactory.newString(readPayload(length));
497+
return ValueFactory.newString(readPayload(length), true);
498498
}
499499
case BINARY: {
500500
int length = unpackBinaryHeader();
501-
return ValueFactory.newBinary(readPayload(length));
501+
return ValueFactory.newBinary(readPayload(length), true);
502502
}
503503
case ARRAY: {
504504
int size = unpackArrayHeader();
505505
Value[] array = new Value[size];
506506
for (int i = 0; i < size; i++) {
507507
array[i] = unpackValue();
508508
}
509-
return ValueFactory.newArray(array);
509+
return ValueFactory.newArray(array, true);
510510
}
511511
case MAP: {
512512
int size = unpackMapHeader();
@@ -517,7 +517,7 @@ public ImmutableValue unpackValue()
517517
kvs[i] = unpackValue();
518518
i++;
519519
}
520-
return ValueFactory.newMap(kvs);
520+
return ValueFactory.newMap(kvs, true);
521521
}
522522
case EXTENSION: {
523523
ExtensionTypeHeader extHeader = unpackExtensionTypeHeader();

‎msgpack-core/src/main/java/org/msgpack/value/ValueFactory.java‎

Lines changed: 77 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -87,12 +87,32 @@ public static ImmutableFloatValue newFloat(double v)
8787

8888
public static ImmutableBinaryValue newBinary(byte[] b)
8989
{
90-
return new ImmutableBinaryValueImpl(b);
90+
return newBinary(b, false);
91+
}
92+
93+
public static ImmutableBinaryValue newBinary(byte[] b, boolean ref)
94+
{
95+
if (ref) {
96+
return new ImmutableBinaryValueImpl(b);
97+
}
98+
else {
99+
return new ImmutableBinaryValueImpl(Arrays.copyOf(b, b.length));
100+
}
91101
}
92102

93103
public static ImmutableBinaryValue newBinary(byte[] b, int off, int len)
94104
{
95-
return new ImmutableBinaryValueImpl(Arrays.copyOfRange(b, off, len));
105+
return newBinary(b, off, len, false);
106+
}
107+
108+
public static ImmutableBinaryValue newBinary(byte[] b, int off, int len, boolean ref)
109+
{
110+
if (ref && off == 0 && len == b.length) {
111+
return new ImmutableBinaryValueImpl(b);
112+
}
113+
else {
114+
return new ImmutableBinaryValueImpl(Arrays.copyOfRange(b, off, len));
115+
}
96116
}
97117

98118
public static ImmutableStringValue newString(String s)
@@ -105,9 +125,29 @@ public static ImmutableStringValue newString(byte[] b)
105125
return new ImmutableStringValueImpl(b);
106126
}
107127

128+
public static ImmutableStringValue newString(byte[] b, boolean ref)
129+
{
130+
if (ref) {
131+
return new ImmutableStringValueImpl(b);
132+
}
133+
else {
134+
return new ImmutableStringValueImpl(Arrays.copyOf(b, b.length));
135+
}
136+
}
137+
108138
public static ImmutableStringValue newString(byte[] b, int off, int len)
109139
{
110-
return new ImmutableStringValueImpl(Arrays.copyOfRange(b, off, len));
140+
return newString(b, off, len, false);
141+
}
142+
143+
public static ImmutableStringValue newString(byte[] b, int off, int len, boolean ref)
144+
{
145+
if (ref && off == 0 && len == b.length) {
146+
return new ImmutableStringValueImpl(b);
147+
}
148+
else {
149+
return new ImmutableStringValueImpl(Arrays.copyOfRange(b, off, len));
150+
}
111151
}
112152

113153
public static ImmutableArrayValue newArray(List<? extends Value> list)
@@ -124,7 +164,22 @@ public static ImmutableArrayValue newArray(Value... array)
124164
if (array.length == 0) {
125165
return ImmutableArrayValueImpl.empty();
126166
}
127-
return new ImmutableArrayValueImpl(Arrays.copyOf(array, array.length));
167+
else {
168+
return new ImmutableArrayValueImpl(Arrays.copyOf(array, array.length));
169+
}
170+
}
171+
172+
public static ImmutableArrayValue newArray(Value[] array, boolean ref)
173+
{
174+
if (array.length == 0) {
175+
return ImmutableArrayValueImpl.empty();
176+
}
177+
else if (ref) {
178+
return new ImmutableArrayValueImpl(array);
179+
}
180+
else {
181+
return new ImmutableArrayValueImpl(Arrays.copyOf(array, array.length));
182+
}
128183
}
129184

130185
public static ImmutableArrayValue emptyArray()
@@ -145,15 +200,30 @@ ImmutableMapValue newMap(Map<K, V> map)
145200
kvs[index] = pair.getValue();
146201
index++;
147202
}
148-
return newMap(kvs);
203+
return new ImmutableMapValueImpl(kvs);
149204
}
150205

151-
public static ImmutableMapValue newMap(Value[] kvs)
206+
public static ImmutableMapValue newMap(Value... kvs)
152207
{
153208
if (kvs.length == 0) {
154209
return ImmutableMapValueImpl.empty();
155210
}
156-
return new ImmutableMapValueImpl(Arrays.copyOf(kvs, kvs.length));
211+
else {
212+
return new ImmutableMapValueImpl(Arrays.copyOf(kvs, kvs.length));
213+
}
214+
}
215+
216+
public static ImmutableMapValue newMap(Value[] kvs, boolean ref)
217+
{
218+
if (kvs.length == 0) {
219+
return ImmutableMapValueImpl.empty();
220+
}
221+
else if (ref) {
222+
return new ImmutableMapValueImpl(kvs);
223+
}
224+
else {
225+
return new ImmutableMapValueImpl(Arrays.copyOf(kvs, kvs.length));
226+
}
157227
}
158228

159229
public static ImmutableMapValue emptyMap()

0 commit comments

Comments
 (0)