Skip to content

Commit

Permalink
Reimplement RepeatedField<T> using an array as the backing store.
Browse files Browse the repository at this point in the history
This is effectively reimplementing List<T>, but with a few advantages:
- We know that an empty repeated field is common, so don't allocate an array until we need to
- With direct access to the array, we can easily convert enum values to int without boxing
- We can relax the restrictions over what happens if the repeated field is modified while iterating, avoiding so much checking

This is somewhat risky, in that reimplementing a building block like this is *always* risky, but hey...
(The performance benefits are significant...)
  • Loading branch information
jskeet committed Jun 12, 2015
1 parent 5a33827 commit 7532f02
Show file tree
Hide file tree
Showing 5 changed files with 231 additions and 51 deletions.
4 changes: 2 additions & 2 deletions csharp/src/ProtocolBuffers.Test/CodedInputStreamTest.cs
Expand Up @@ -487,7 +487,7 @@ public void TestNegativeEnumPackedArray()
uint tag;
Assert.IsTrue(input.ReadTag(out tag));

List<TestNegEnum> values = new List<TestNegEnum>();
RepeatedField<TestNegEnum> values = new RepeatedField<TestNegEnum>();
input.ReadEnumArray(tag, values);

Assert.AreEqual(6, values.Count);
Expand All @@ -511,7 +511,7 @@ public void TestNegativeEnumArray()
uint tag;
Assert.IsTrue(input.ReadTag(out tag));

List<TestNegEnum> values = new List<TestNegEnum>();
RepeatedField<TestNegEnum> values = new RepeatedField<TestNegEnum>();
input.ReadEnumArray(tag, values);

Assert.AreEqual(6, values.Count);
Expand Down
9 changes: 5 additions & 4 deletions csharp/src/ProtocolBuffers.Test/RepeatedFieldTest.cs
Expand Up @@ -40,11 +40,12 @@ public void Add_Sequence()
[Test]
public void Add_RepeatedField()
{
var list = new RepeatedField<string>();
var list = new RepeatedField<string> { "original" };
list.Add(new RepeatedField<string> { "foo", "bar" });
Assert.AreEqual(2, list.Count);
Assert.AreEqual("foo", list[0]);
Assert.AreEqual("bar", list[1]);
Assert.AreEqual(3, list.Count);
Assert.AreEqual("original", list[0]);
Assert.AreEqual("foo", list[1]);
Assert.AreEqual("bar", list[2]);
}
}
}
7 changes: 4 additions & 3 deletions csharp/src/ProtocolBuffers/CodedInputStream.cs
Expand Up @@ -38,6 +38,7 @@
using System.Collections.Generic;
using System.IO;
using System.Text;
using Google.Protobuf.Collections;
using Google.Protobuf.Descriptors;

namespace Google.Protobuf
Expand Down Expand Up @@ -700,7 +701,7 @@ public void ReadFloatArray(uint fieldTag, ICollection<float> list)
}
}

public void ReadEnumArray<T>(uint fieldTag, ICollection<T> list)
public void ReadEnumArray<T>(uint fieldTag, RepeatedField<T> list)
where T : struct, IComparable, IFormattable
{
WireFormat.WireType wformat = WireFormat.GetTagWireType(fieldTag);
Expand All @@ -712,8 +713,8 @@ public void ReadEnumArray<T>(uint fieldTag, ICollection<T> list)
int limit = PushLimit(length);
while (!ReachedLimit)
{
// TODO(jonskeet): Avoid this horrible boxing!
list.Add((T)(object) ReadEnum());
// Ghastly hack, but it works...
list.AddInt32(ReadEnum());
}
PopLimit(limit);
}
Expand Down
23 changes: 14 additions & 9 deletions csharp/src/ProtocolBuffers/CodedOutputStream.cs
Expand Up @@ -743,10 +743,11 @@ public void WriteEnumArray<T>(int fieldNumber, RepeatedField<T> list)
{
return;
}
// TODO(jonskeet): Avoid the Cast call here. Work out a better mass "T to int" conversion.
foreach (int value in list.Cast<int>())
// Bit of a hack, to access the values as ints
var iterator = list.GetInt32Enumerator();
while (iterator.MoveNext())
{
WriteEnum(fieldNumber, value);
WriteEnum(fieldNumber, iterator.Current);
}
}

Expand Down Expand Up @@ -956,15 +957,19 @@ public void WritePackedEnumArray<T>(int fieldNumber, RepeatedField<T> list)
{
return;
}
// Obviously, we'll want to get rid of this hack...
var temporaryHack = new RepeatedField<int>();
temporaryHack.Add(list.Cast<int>());
uint size = temporaryHack.CalculateSize(ComputeEnumSizeNoTag);
// Bit of a hack, to access the values as ints
var iterator = list.GetInt32Enumerator();
uint size = 0;
while (iterator.MoveNext())
{
size += (uint) ComputeEnumSizeNoTag(iterator.Current);
}
iterator.Reset();
WriteTag(fieldNumber, WireFormat.WireType.LengthDelimited);
WriteRawVarint32(size);
foreach (int value in temporaryHack)
while (iterator.MoveNext())
{
WriteEnumNoTag(value);
WriteEnumNoTag(iterator.Current);
}
}

Expand Down

0 comments on commit 7532f02

Please sign in to comment.