From 6e2536f5859b4838fee9af4ef89372921cc9beb7 Mon Sep 17 00:00:00 2001 From: Jens Johansen Date: Tue, 1 May 2018 09:35:21 +0000 Subject: [PATCH] [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 Reviewed-by: Kevin Millikin Reviewed-by: Vyacheslav Egorov --- pkg/kernel/binary.md | 6 ++++-- pkg/kernel/lib/binary/ast_from_binary.dart | 20 +++++++++++++++++-- pkg/kernel/lib/binary/ast_to_binary.dart | 19 ++++++++++++++---- pkg/kernel/lib/binary/tag.dart | 2 +- .../frontend/kernel_binary_flowgraph.cc | 17 ++++++++-------- .../frontend/kernel_binary_flowgraph.h | 1 + runtime/vm/kernel_binary.h | 10 +++++++++- runtime/vm/kernel_loader.cc | 3 +-- 8 files changed, 58 insertions(+), 20 deletions(-) diff --git a/pkg/kernel/binary.md b/pkg/kernel/binary.md index f87de3dd7a5..87adfe2ce7b 100644 --- a/pkg/kernel/binary.md +++ b/pkg/kernel/binary.md @@ -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 { 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 { diff --git a/pkg/kernel/lib/binary/ast_from_binary.dart b/pkg/kernel/lib/binary/ast_from_binary.dart index bfdf151720d..db2d0c41354 100644 --- a/pkg/kernel/lib/binary/ast_from_binary.dart +++ b/pkg/kernel/lib/binary/ast_from_binary.dart @@ -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 readByteList() { List 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: diff --git a/pkg/kernel/lib/binary/ast_to_binary.dart b/pkg/kernel/lib/binary/ast_to_binary.dart index 3c8dc01efa1..9bcaed9b22c 100644 --- a/pkg/kernel/lib/binary/ast_to_binary.dart +++ b/pkg/kernel/lib/binary/ast_to_binary.dart @@ -147,7 +147,7 @@ class BinaryPrinter implements Visitor, 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, 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) { diff --git a/pkg/kernel/lib/binary/tag.dart b/pkg/kernel/lib/binary/tag.dart index bc702038d20..877657fac61 100644 --- a/pkg/kernel/lib/binary/tag.dart +++ b/pkg/kernel/lib/binary/tag.dart @@ -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 { diff --git a/runtime/vm/compiler/frontend/kernel_binary_flowgraph.cc b/runtime/vm/compiler/frontend/kernel_binary_flowgraph.cc index 2ab38408780..bf85dd9ff31 100644 --- a/runtime/vm/compiler/frontend/kernel_binary_flowgraph.cc +++ b/runtime/vm/compiler/frontend/kernel_binary_flowgraph.cc @@ -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; } diff --git a/runtime/vm/compiler/frontend/kernel_binary_flowgraph.h b/runtime/vm/compiler/frontend/kernel_binary_flowgraph.h index b8033bdc0b4..42b6c857b63 100644 --- a/runtime/vm/compiler/frontend/kernel_binary_flowgraph.h +++ b/runtime/vm/compiler/frontend/kernel_binary_flowgraph.h @@ -1049,6 +1049,7 @@ class KernelReaderHelper { uint32_t ReadUInt(); uint32_t ReadUInt32(); uint32_t PeekUInt(); + double ReadDouble(); uint32_t PeekListLength(); StringIndex ReadStringReference(); NameIndex ReadCanonicalNameReference(); diff --git a/runtime/vm/kernel_binary.h b/runtime/vm/kernel_binary.h index e426f4dd0c5..daee5401a7d 100644 --- a/runtime/vm/kernel_binary.h +++ b/runtime/vm/kernel_binary.h @@ -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(&this->buffer()[offset_])); + offset_ += 8; + return value; + } + uint32_t ReadUInt() { ASSERT((size_ >= 1) && (offset_ >= 0) && (offset_ <= size_ - 1)); diff --git a/runtime/vm/kernel_loader.cc b/runtime/vm/kernel_loader.cc index 4b832122c3a..6a3358cc2ca 100644 --- a/runtime/vm/kernel_loader.cc +++ b/runtime/vm/kernel_loader.cc @@ -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: