[kernel] Change dill representation of doubles

Previously doubles was saved as strings in the string table,
with a DoubleLiteral holding a StringReference.

This can cause overhead in both computation time
(converting the double to and from string) as well as size
(e.g. a single usage of the previously unused double 1000000.42
would use (at least)
* 10 bytes for the characters
* 1 byte for the size
* 1 byte for the reference to the string

whereas saving it as a double would simply save the 8 bytes.

On the other hand the string table doesn't contain duplicates so
many usages of the same double will use more space.

The SDK dill file size decreases slightly (< 1 KB).

On a Dart file with 1M different doubles (0.42, 1.42, ..., 999999.42)
added to a list:

Before:

compile and write via fasta (non-strong-mode): 0:12.18
Reading (via dart, eager): 2500-2600 ms
Writing (to null sink) (after reading): 1600-1800 ms
Output dill file (via fasta compile): ~62 MB

After:

compile and write via fasta (non-strong-mode): 0:11.76
Reading (via dart, eager): 2050-2350 ms
Writing (to null sink) (after reading): 400-550 ms
Output dill file (via fasta compile): ~54 MB

Running the dill file is ~the same time, but "Maximum resident set size
(kbytes)" (from /usr/bin/time -v) decreases with ~4%.

On the other side, if it's 1M of the same doubles (0.42), while
compiling is ~the same speed, the output dill goes from 43MB to 50MB.
Surprisingly the "Maximum resident set size (kbytes)" still decreases
though (~3%).

Running flutter test in flutter/packages/flutter:

Before:
```
02:33 +2425 ~18: All tests passed!
02:28 +2425 ~18: All tests passed!
02:28 +2425 ~18: All tests passed!
```

After:
```
02:12 +2425 ~18: All tests passed!
02:11 +2425 ~18: All tests passed!
02:12 +2425 ~18: All tests passed!
```

So that's -12.0267% +/- 3.15253%

File size of a dill file of an arbitrary test using flutter is reduced by ~44 KB (~0.3%).

Change-Id: I64151376cde1dae6f0d02b3d96991bc432a994ae
Reviewed-on: https://dart-review.googlesource.com/41660
Commit-Queue: Jens Johansen <jensj@google.com>
Reviewed-by: Kevin Millikin <kmillikin@google.com>
Reviewed-by: Vyacheslav Egorov <vegorov@google.com>
This commit is contained in:
Jens Johansen 2018-05-01 09:35:21 +00:00 committed by commit-bot@chromium.org
parent b1611684fe
commit 6e2536f585
8 changed files with 58 additions and 20 deletions

View file

@ -42,6 +42,8 @@ type UInt30 extends UInt {
type UInt32 = big endian 32-bit unsigned integer
type Double = Double-precision floating-point number.
type List<T> {
UInt length;
T[length] items;
@ -712,7 +714,7 @@ type BigIntLiteral extends Expression {
type DoubleLiteral extends Expression {
Byte tag = 40;
StringReference valueString;
Double value;
}
type TrueLiteral extends Expression {
@ -875,7 +877,7 @@ type IntConstant extends Constant {
type DoubleConstant extends Constant {
Byte tag = 3;
StringReference value;
Double value;
}
type StringConstant extends Constant {

View file

@ -97,6 +97,22 @@ class BinaryBuilder {
readByte();
}
final Float64List _doubleBuffer = new Float64List(1);
Uint8List _doubleBufferUint8;
double readDouble() {
_doubleBufferUint8 ??= _doubleBuffer.buffer.asUint8List();
_doubleBufferUint8[0] = readByte();
_doubleBufferUint8[1] = readByte();
_doubleBufferUint8[2] = readByte();
_doubleBufferUint8[3] = readByte();
_doubleBufferUint8[4] = readByte();
_doubleBufferUint8[5] = readByte();
_doubleBufferUint8[6] = readByte();
_doubleBufferUint8[7] = readByte();
return _doubleBuffer[0];
}
List<int> readByteList() {
List<int> bytes = new Uint8List(readUInt());
bytes.setRange(0, bytes.length, _bytes, _byteOffset);
@ -174,7 +190,7 @@ class BinaryBuilder {
case ConstantTag.IntConstant:
return new IntConstant((readExpression() as IntLiteral).value);
case ConstantTag.DoubleConstant:
return new DoubleConstant(double.parse(readStringReference()));
return new DoubleConstant(readDouble());
case ConstantTag.StringConstant:
return new StringConstant(readStringReference());
case ConstantTag.MapConstant:
@ -1337,7 +1353,7 @@ class BinaryBuilder {
case Tag.BigIntLiteral:
return new IntLiteral(int.parse(readStringReference()));
case Tag.DoubleLiteral:
return new DoubleLiteral(double.parse(readStringReference()));
return new DoubleLiteral(readDouble());
case Tag.TrueLiteral:
return new BoolLiteral(true);
case Tag.FalseLiteral:

View file

@ -147,7 +147,7 @@ class BinaryPrinter implements Visitor<void>, BinarySink {
writeInteger(constant.value);
} else if (constant is DoubleConstant) {
writeByte(ConstantTag.DoubleConstant);
writeStringReference('${constant.value}');
writeDouble(constant.value);
} else if (constant is StringConstant) {
writeByte(ConstantTag.StringConstant);
writeStringReference(constant.value);
@ -1240,13 +1240,12 @@ class BinaryPrinter implements Visitor<void>, BinarySink {
@override
void visitDoubleLiteral(DoubleLiteral node) {
writeByte(Tag.DoubleLiteral);
writeDouble(node.value);
}
writeDouble(double value) {
// TODO: Pick a better format for double literals.
writeByte(Tag.DoubleLiteral);
writeStringReference('$value');
_sink.addDouble(value);
}
@override
@ -2193,8 +2192,20 @@ class BufferedSink {
int length = 0;
int flushedLength = 0;
Float64List _doubleBuffer = new Float64List(1);
Uint8List _doubleBufferUint8;
BufferedSink(this._sink);
void addDouble(double d) {
_doubleBufferUint8 ??= _doubleBuffer.buffer.asUint8List();
_doubleBuffer[0] = d;
addByte4(_doubleBufferUint8[0], _doubleBufferUint8[1],
_doubleBufferUint8[2], _doubleBufferUint8[3]);
addByte4(_doubleBufferUint8[4], _doubleBufferUint8[5],
_doubleBufferUint8[6], _doubleBufferUint8[7]);
}
void addByte(int byte) {
_buffer[length++] = byte;
if (length == SIZE) {

View file

@ -135,7 +135,7 @@ class Tag {
/// Internal version of kernel binary format.
/// Bump it when making incompatible changes in kernel binaries.
/// Keep in sync with runtime/vm/kernel_binary.h.
static const int BinaryFormatVersion = 4;
static const int BinaryFormatVersion = 5;
}
abstract class ConstantTag {

View file

@ -1552,7 +1552,7 @@ void StreamingScopeBuilder::VisitExpression() {
builder_->ReadUInt(); // read value.
return;
case kDoubleLiteral:
builder_->SkipStringReference(); // read index into string table.
builder_->ReadDouble(); // read value.
return;
case kTrueLiteral:
return;
@ -3546,8 +3546,7 @@ void StreamingConstantEvaluator::EvaluateIntLiteral(bool is_negative) {
}
void StreamingConstantEvaluator::EvaluateDoubleLiteral() {
result_ = Double::New(H.DartString(builder_->ReadStringReference()),
Heap::kOld); // read string reference.
result_ = Double::New(builder_->ReadDouble(), Heap::kOld); // read value.
result_ = H.Canonicalize(result_);
}
@ -6027,6 +6026,10 @@ uint32_t KernelReaderHelper::PeekUInt() {
return reader_.ReadUInt();
}
double KernelReaderHelper::ReadDouble() {
return reader_.ReadDouble();
}
uint32_t KernelReaderHelper::PeekListLength() {
AlternativeReadingScope alt(&reader_);
return reader_.ReadListLength();
@ -6494,7 +6497,7 @@ void KernelReaderHelper::SkipExpression() {
ReadUInt(); // read value.
return;
case kDoubleLiteral:
SkipStringReference(); // read index into string table.
ReadDouble(); // read value.
return;
case kTrueLiteral:
return;
@ -8987,8 +8990,7 @@ Fragment StreamingFlowGraphBuilder::BuildDoubleLiteral(
if (position != NULL) *position = TokenPosition::kNoSource;
Double& constant = Double::ZoneHandle(
Z, Double::NewCanonical(
H.DartString(ReadStringReference()))); // read string reference.
Z, Double::NewCanonical(ReadDouble())); // read double.
return Constant(constant);
}
@ -10822,8 +10824,7 @@ const Array& ConstantHelper::ReadConstantTable() {
break;
}
case kDoubleConstant: {
temp_instance_ = Double::New(
H.DartString(builder_.ReadStringReference()), Heap::kOld);
temp_instance_ = Double::New(builder_.ReadDouble(), Heap::kOld);
temp_instance_ = H.Canonicalize(temp_instance_);
break;
}

View file

@ -1049,6 +1049,7 @@ class KernelReaderHelper {
uint32_t ReadUInt();
uint32_t ReadUInt32();
uint32_t PeekUInt();
double ReadDouble();
uint32_t PeekListLength();
StringIndex ReadStringReference();
NameIndex ReadCanonicalNameReference();

View file

@ -19,7 +19,7 @@ namespace kernel {
// Keep in sync with package:kernel/lib/binary/tag.dart.
static const uint32_t kMagicProgramFile = 0x90ABCDEFu;
static const uint32_t kBinaryFormatVersion = 4;
static const uint32_t kBinaryFormatVersion = 5;
// Keep in sync with package:kernel/lib/binary/tag.dart
#define KERNEL_TAG_LIST(V) \
@ -213,6 +213,14 @@ class Reader : public ValueObject {
return value;
}
double ReadDouble() {
ASSERT((size_ >= 8) && (offset_ >= 0) && (offset_ <= size_ - 8));
double value = ReadUnaligned(
reinterpret_cast<const double*>(&this->buffer()[offset_]));
offset_ += 8;
return value;
}
uint32_t ReadUInt() {
ASSERT((size_ >= 1) && (offset_ >= 0) && (offset_ <= size_ - 1));

View file

@ -82,8 +82,7 @@ class SimpleExpressionConverter {
return true;
case kDoubleLiteral:
simple_value_ = &Double::ZoneHandle(
Z, Double::New(H.DartString(builder_->ReadStringReference()),
Heap::kOld)); // read string reference.
Z, Double::New(builder_->ReadDouble(), Heap::kOld)); // read value.
*simple_value_ = H.Canonicalize(*simple_value_);
return true;
case kTrueLiteral: