From 03a468f83273c85878351c1b79a8414785643794 Mon Sep 17 00:00:00 2001 From: Frank Emrich Date: Thu, 4 Feb 2021 13:49:13 +0100 Subject: [PATCH] [dict-proto] Allow storing certain PropertyDetails in single byte MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This CL adds PropertyDetails::ToByte and ::FromByte. These are not applicable to all PropertDetails, but only those for dictionary-backed properties with an (unused) enumeration index with value 0. The motivation for this is that those dictionare backing stores that don't store the enumeration order in the PropertyDetails but store it in the table itself (like OrderedNameDictionary and the upcoming SwissNameDictionary), can store PropertyDetails in an array of bytes. Bug: v8:11388 Change-Id: Id346b924cd7c67b2f33cbc7a7807eec31cefbeec Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2672029 Commit-Queue: Frank Emrich Reviewed-by: Igor Sheludko Reviewed-by: Marja Hölttä Cr-Commit-Position: refs/heads/master@{#72528} --- src/objects/property-details.h | 61 +++++++++++++++++++++-- test/cctest/BUILD.gn | 1 + test/cctest/test-property-details.cc | 72 ++++++++++++++++++++++++++++ 3 files changed, 129 insertions(+), 5 deletions(-) create mode 100644 test/cctest/test-property-details.cc diff --git a/src/objects/property-details.h b/src/objects/property-details.h index 884df4059c..b2925927d0 100644 --- a/src/objects/property-details.h +++ b/src/objects/property-details.h @@ -211,16 +211,14 @@ static const int kInvalidEnumCacheSentinel = // A PropertyCell's property details contains a cell type that is meaningful if // the cell is still valid (does not hold the hole). enum class PropertyCellType { + kMutable, // Cell will no longer be tracked as constant. kUndefined, // The PREMONOMORPHIC of property cells. kConstant, // Cell has been assigned only once. kConstantType, // Cell has been assigned only one type. - kMutable, // Cell will no longer be tracked as constant. - // Arbitrary choice for dictionaries not holding cells. + // Value for dictionaries not holding cells, must have value 0: kNoCell = kMutable, }; -std::ostream& operator<<(std::ostream& os, PropertyCellType type); - // PropertyDetails captures type and attributes for a property. // They are used both in property dictionaries and instance descriptors. class PropertyDetails { @@ -343,6 +341,8 @@ class PropertyDetails { return PropertyCellTypeField::decode(value_); } + bool operator==(const PropertyDetails& b) const { return value_ == b.value_; } + // Bit fields in value_ (type, shift, size). Must be public so the // constants can be embedded in generated code. using KindField = base::BitField; @@ -356,7 +356,7 @@ class PropertyDetails { static const int kAttributesDontEnumMask = (DONT_ENUM << AttributesField::kShift); - // Bit fields for normalized objects. + // Bit fields for normalized/dictionary mode objects. using PropertyCellTypeField = AttributesField::Next; using DictionaryStorageField = PropertyCellTypeField::Next; @@ -371,6 +371,17 @@ class PropertyDetails { STATIC_ASSERT(DictionaryStorageField::kLastUsedBit < 31); STATIC_ASSERT(FieldIndexField::kLastUsedBit < 31); + // DictionaryStorageField must be the last field, so that overflowing it + // doesn't overwrite other fields. + STATIC_ASSERT(DictionaryStorageField::kLastUsedBit == 30); + + // All bits for non-global dictionary mode objects except enumeration index + // must fit in a byte. + STATIC_ASSERT(KindField::kLastUsedBit < 8); + STATIC_ASSERT(ConstnessField::kLastUsedBit < 8); + STATIC_ASSERT(AttributesField::kLastUsedBit < 8); + STATIC_ASSERT(LocationField::kLastUsedBit < 8); + static const int kInitialIndex = 1; static constexpr PropertyConstness kConstIfDictConstnessTracking = @@ -395,6 +406,42 @@ class PropertyDetails { void PrintAsSlowTo(std::ostream& out, bool print_dict_index); void PrintAsFastTo(std::ostream& out, PrintMode mode = kPrintFull); + // Encodes those property details for non-global dictionary properties + // with an enumeration index of 0 as a single byte. + uint8_t ToByte() { + // We only care about the value of KindField, ConstnessField, and + // AttributesField. LocationField is also stored, but it will always be + // kField. We've statically asserted earlier that all those fields fit into + // a byte together. + + // PropertyCellTypeField comes next, its value must be kNoCell == 0 for + // dictionary mode PropertyDetails anyway. + DCHECK_EQ(PropertyCellType::kNoCell, cell_type()); + STATIC_ASSERT(static_cast(PropertyCellType::kNoCell) == 0); + + // Only to be used when the enum index isn't actually maintained + // by the PropertyDetails: + DCHECK_EQ(0, dictionary_index()); + + return value_; + } + + // Only to be used for bytes obtained by ToByte. In particular, only used for + // non-global dictionary properties. + static PropertyDetails FromByte(uint8_t encoded_details) { + // The 0-extension to 32bit sets PropertyCellType to kNoCell and + // enumeration index to 0, as intended. Everything else is obtained from + // |encoded_details|. + + PropertyDetails details(encoded_details); + + DCHECK_EQ(0, details.dictionary_index()); + DCHECK_EQ(PropertyLocation::kField, details.location()); + DCHECK_EQ(PropertyCellType::kNoCell, details.cell_type()); + + return details; + } + private: PropertyDetails(int value, int pointer) { value_ = DescriptorPointer::update(value, pointer); @@ -410,6 +457,8 @@ class PropertyDetails { value_ = AttributesField::update(value, attributes); } + explicit PropertyDetails(uint32_t value) : value_{value} {} + uint32_t value_; }; @@ -434,6 +483,8 @@ V8_EXPORT_PRIVATE std::ostream& operator<<( std::ostream& os, const PropertyAttributes& attributes); V8_EXPORT_PRIVATE std::ostream& operator<<(std::ostream& os, PropertyConstness constness); +V8_EXPORT_PRIVATE std::ostream& operator<<(std::ostream& os, + PropertyCellType type); } // namespace internal } // namespace v8 diff --git a/test/cctest/BUILD.gn b/test/cctest/BUILD.gn index 91c4483a66..9f5b106587 100644 --- a/test/cctest/BUILD.gn +++ b/test/cctest/BUILD.gn @@ -264,6 +264,7 @@ v8_source_set("cctest_sources") { "test-persistent-handles.cc", "test-platform.cc", "test-profile-generator.cc", + "test-property-details.cc", "test-random-number-generator.cc", "test-regexp.cc", "test-representation.cc", diff --git a/test/cctest/test-property-details.cc b/test/cctest/test-property-details.cc new file mode 100644 index 0000000000..2eb2499a70 --- /dev/null +++ b/test/cctest/test-property-details.cc @@ -0,0 +1,72 @@ +// Copyright 2021 the V8 project authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include + +#include "src/objects/property-details.h" +#include "test/cctest/cctest.h" + +namespace v8 { +namespace internal { + +namespace { + +std::vector make_details() { + std::vector result; + for (PropertyKind kind : {PropertyKind::kData, PropertyKind::kAccessor}) { + for (PropertyConstness constness : + {PropertyConstness::kConst, PropertyConstness::kMutable}) { + for (PropertyCellType cell_type : + {PropertyCellType::kConstant, PropertyCellType::kConstantType, + PropertyCellType::kMutable, PropertyCellType::kUndefined, + PropertyCellType::kNoCell}) { + for (int attrs = 0; attrs < 8; ++attrs) { + PropertyAttributes attributes = + static_cast(attrs); + PropertyDetails details(kind, attributes, cell_type); + details = details.CopyWithConstness(constness); + result.push_back(details); + } + } + } + } + return result; +} + +} // namespace + +#ifndef DEBUG +// This test will trigger a DCHECK failure in debug mode. We must ensure that in +// release mode, the enum index doesn't interfere with other fields once it +// becomes too large. +TEST(ExceedMaxEnumerationIndex) { + int too_large_enum_index = std::numeric_limits::max(); + + for (PropertyDetails d : make_details()) { + PropertyDetails copy(d); + + d = d.set_index(too_large_enum_index); + CHECK_EQ(copy.kind(), d.kind()); + CHECK_EQ(copy.location(), d.location()); + CHECK_EQ(copy.attributes(), d.attributes()); + CHECK_EQ(copy.cell_type(), d.cell_type()); + CHECK_EQ(PropertyDetails::DictionaryStorageField::kMax, + d.dictionary_index()); + } +} +#endif + +TEST(AsByte) { + for (PropertyDetails original : make_details()) { + if (original.cell_type() != PropertyCellType::kNoCell) continue; + + uint8_t as_byte = original.ToByte(); + PropertyDetails from_byte = PropertyDetails::FromByte(as_byte); + + CHECK_EQ(original, from_byte); + } +} + +} // namespace internal +} // namespace v8