Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Inform the Dart VM when snapshots are safe to use with madvise(DONTNEED). #29320

Merged
merged 1 commit into from
Oct 28, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 30 additions & 0 deletions fml/file_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -269,6 +269,36 @@ TEST(FileTest, EmptyMappingTest) {
ASSERT_TRUE(fml::UnlinkFile(dir.fd(), "my_contents"));
}

TEST(FileTest, MappingDontNeedSafeTest) {
fml::ScopedTemporaryDirectory dir;

{
auto file = fml::OpenFile(dir.fd(), "my_contents", true,
fml::FilePermission::kReadWrite);
WriteStringToFile(file, "some content");
}

{
auto file = fml::OpenFile(dir.fd(), "my_contents", false,
fml::FilePermission::kRead);
fml::FileMapping mapping(file);
ASSERT_TRUE(mapping.IsValid());
ASSERT_EQ(mapping.GetMutableMapping(), nullptr);
ASSERT_TRUE(mapping.IsDontNeedSafe());
}

{
auto file = fml::OpenFile(dir.fd(), "my_contents", false,
fml::FilePermission::kReadWrite);
fml::FileMapping mapping(file, {fml::FileMapping::Protection::kRead,
fml::FileMapping::Protection::kWrite});
ASSERT_TRUE(mapping.IsValid());
ASSERT_NE(mapping.GetMutableMapping(), nullptr);
ASSERT_FALSE(mapping.IsDontNeedSafe());
}
ASSERT_TRUE(fml::UnlinkFile(dir.fd(), "my_contents"));
}

TEST(FileTest, FileTestsWork) {
fml::ScopedTemporaryDirectory dir;
ASSERT_TRUE(dir.fd().is_valid());
Expand Down
24 changes: 22 additions & 2 deletions fml/mapping.cc
Original file line number Diff line number Diff line change
Expand Up @@ -81,11 +81,19 @@ const uint8_t* DataMapping::GetMapping() const {
return data_.data();
}

bool DataMapping::IsDontNeedSafe() const {
return false;
}

// NonOwnedMapping
NonOwnedMapping::NonOwnedMapping(const uint8_t* data,
size_t size,
const ReleaseProc& release_proc)
: data_(data), size_(size), release_proc_(release_proc) {}
const ReleaseProc& release_proc,
bool dontneed_safe)
: data_(data),
size_(size),
release_proc_(release_proc),
dontneed_safe_(dontneed_safe) {}

NonOwnedMapping::~NonOwnedMapping() {
if (release_proc_) {
Expand All @@ -101,6 +109,10 @@ const uint8_t* NonOwnedMapping::GetMapping() const {
return data_;
}

bool NonOwnedMapping::IsDontNeedSafe() const {
return dontneed_safe_;
}

// MallocMapping
MallocMapping::MallocMapping() : data_(nullptr), size_(0) {}

Expand Down Expand Up @@ -134,6 +146,10 @@ const uint8_t* MallocMapping::GetMapping() const {
return data_;
}

bool MallocMapping::IsDontNeedSafe() const {
return false;
}

uint8_t* MallocMapping::Release() {
uint8_t* result = data_;
data_ = nullptr;
Expand Down Expand Up @@ -174,4 +190,8 @@ const uint8_t* SymbolMapping::GetMapping() const {
return mapping_;
}

bool SymbolMapping::IsDontNeedSafe() const {
return true;
}

} // namespace fml
23 changes: 22 additions & 1 deletion fml/mapping.h
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,10 @@ class Mapping {

virtual const uint8_t* GetMapping() const = 0;

// Whether calling madvise(DONTNEED) on the mapping is non-destructive.
// Generally true for file-mapped memory and false for anonymous memory.
virtual bool IsDontNeedSafe() const = 0;

private:
FML_DISALLOW_COPY_AND_ASSIGN(Mapping);
};
Expand Down Expand Up @@ -65,6 +69,9 @@ class FileMapping final : public Mapping {
// |Mapping|
const uint8_t* GetMapping() const override;

// |Mapping|
bool IsDontNeedSafe() const override;

uint8_t* GetMutableMapping();

bool IsValid() const;
Expand Down Expand Up @@ -96,6 +103,9 @@ class DataMapping final : public Mapping {
// |Mapping|
const uint8_t* GetMapping() const override;

// |Mapping|
bool IsDontNeedSafe() const override;

private:
std::vector<uint8_t> data_;

Expand All @@ -107,7 +117,8 @@ class NonOwnedMapping final : public Mapping {
using ReleaseProc = std::function<void(const uint8_t* data, size_t size)>;
NonOwnedMapping(const uint8_t* data,
size_t size,
const ReleaseProc& release_proc = nullptr);
const ReleaseProc& release_proc = nullptr,
bool dontneed_safe = false);

~NonOwnedMapping() override;

Expand All @@ -117,10 +128,14 @@ class NonOwnedMapping final : public Mapping {
// |Mapping|
const uint8_t* GetMapping() const override;

// |Mapping|
bool IsDontNeedSafe() const override;

private:
const uint8_t* const data_;
const size_t size_;
const ReleaseProc release_proc_;
const bool dontneed_safe_;

FML_DISALLOW_COPY_AND_ASSIGN(NonOwnedMapping);
};
Expand Down Expand Up @@ -162,6 +177,9 @@ class MallocMapping final : public Mapping {
// |Mapping|
const uint8_t* GetMapping() const override;

// |Mapping|
bool IsDontNeedSafe() const override;

/// Removes ownership of the data buffer.
/// After this is called; the mapping will point to nullptr.
[[nodiscard]] uint8_t* Release();
Expand All @@ -186,6 +204,9 @@ class SymbolMapping final : public Mapping {
// |Mapping|
const uint8_t* GetMapping() const override;

// |Mapping|
bool IsDontNeedSafe() const override;

private:
fml::RefPtr<fml::NativeLibrary> native_library_;
const uint8_t* mapping_ = nullptr;
Expand Down
7 changes: 7 additions & 0 deletions fml/mapping_unittests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -52,4 +52,11 @@ TEST(MallocMapping, Release) {
ASSERT_EQ(0u, mapping.GetSize());
}

TEST(MallocMapping, IsDontNeedSafe) {
size_t length = 10;
MallocMapping mapping(reinterpret_cast<uint8_t*>(malloc(length)), length);
ASSERT_NE(nullptr, mapping.GetMapping());
ASSERT_FALSE(mapping.IsDontNeedSafe());
}

} // namespace fml
4 changes: 4 additions & 0 deletions fml/platform/posix/mapping_posix.cc
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,10 @@ const uint8_t* FileMapping::GetMapping() const {
return mapping_;
}

bool FileMapping::IsDontNeedSafe() const {
return mutable_mapping_ == nullptr;
}

bool FileMapping::IsValid() const {
return valid_;
}
Expand Down
4 changes: 4 additions & 0 deletions fml/platform/win/mapping_win.cc
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,10 @@ const uint8_t* FileMapping::GetMapping() const {
return mapping_;
}

bool FileMapping::IsDontNeedSafe() const {
return mutable_mapping_ == nullptr;
}

bool FileMapping::IsValid() const {
return valid_;
}
Expand Down
5 changes: 5 additions & 0 deletions runtime/dart_isolate.cc
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,10 @@ void DartIsolate::Flags::SetNullSafetyEnabled(bool enabled) {
flags_.null_safety = enabled;
}

void DartIsolate::Flags::SetIsDontNeedSafe(bool value) {
flags_.snapshot_is_dontneed_safe = value;
}

Dart_IsolateFlags DartIsolate::Flags::Get() const {
return flags_;
}
Expand Down Expand Up @@ -139,6 +143,7 @@ std::weak_ptr<DartIsolate> DartIsolate::CreateRunningRootIsolate(

isolate_flags.SetNullSafetyEnabled(
isolate_configration->IsNullSafetyEnabled(*isolate_snapshot));
isolate_flags.SetIsDontNeedSafe(isolate_snapshot->IsDontNeedSafe());

auto isolate = CreateRootIsolate(settings, //
isolate_snapshot, //
Expand Down
1 change: 1 addition & 0 deletions runtime/dart_isolate.h
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ class DartIsolate : public UIDartState {
~Flags();

void SetNullSafetyEnabled(bool enabled);
void SetIsDontNeedSafe(bool value);

Dart_IsolateFlags Get() const;

Expand Down
32 changes: 28 additions & 4 deletions runtime/dart_snapshot.cc
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,11 @@ static std::shared_ptr<const fml::Mapping> SearchMapping(
static std::shared_ptr<const fml::Mapping> ResolveVMData(
const Settings& settings) {
#if DART_SNAPSHOT_STATIC_LINK
return std::make_unique<fml::NonOwnedMapping>(kDartVmSnapshotData, 0);
return std::make_unique<fml::NonOwnedMapping>(kDartVmSnapshotData,
0, // size
nullptr, // release_func
true // dontneed_safe
);
#else // DART_SNAPSHOT_STATIC_LINK
return SearchMapping(
settings.vm_snapshot_data, // embedder_mapping_callback
Expand All @@ -112,7 +116,11 @@ static std::shared_ptr<const fml::Mapping> ResolveVMData(
static std::shared_ptr<const fml::Mapping> ResolveVMInstructions(
const Settings& settings) {
#if DART_SNAPSHOT_STATIC_LINK
return std::make_unique<fml::NonOwnedMapping>(kDartVmSnapshotInstructions, 0);
return std::make_unique<fml::NonOwnedMapping>(kDartVmSnapshotInstructions,
0, // size
nullptr, // release_func
true // dontneed_safe
);
#else // DART_SNAPSHOT_STATIC_LINK
return SearchMapping(
settings.vm_snapshot_instr, // embedder_mapping_callback
Expand All @@ -127,7 +135,11 @@ static std::shared_ptr<const fml::Mapping> ResolveVMInstructions(
static std::shared_ptr<const fml::Mapping> ResolveIsolateData(
const Settings& settings) {
#if DART_SNAPSHOT_STATIC_LINK
return std::make_unique<fml::NonOwnedMapping>(kDartIsolateSnapshotData, 0);
return std::make_unique<fml::NonOwnedMapping>(kDartIsolateSnapshotData,
0, // size
nullptr, // release_func
true // dontneed_safe
);
#else // DART_SNAPSHOT_STATIC_LINK
return SearchMapping(
settings.isolate_snapshot_data, // embedder_mapping_callback
Expand All @@ -143,7 +155,11 @@ static std::shared_ptr<const fml::Mapping> ResolveIsolateInstructions(
const Settings& settings) {
#if DART_SNAPSHOT_STATIC_LINK
return std::make_unique<fml::NonOwnedMapping>(
kDartIsolateSnapshotInstructions, 0);
kDartIsolateSnapshotInstructions,
0, // size
nullptr, // release_func
true // dontneed_safe
);
#else // DART_SNAPSHOT_STATIC_LINK
return SearchMapping(
settings.isolate_snapshot_instr, // embedder_mapping_callback
Expand Down Expand Up @@ -233,6 +249,14 @@ const uint8_t* DartSnapshot::GetInstructionsMapping() const {
return instructions_ ? instructions_->GetMapping() : nullptr;
}

bool DartSnapshot::IsDontNeedSafe() const {
if (data_ && !data_->IsDontNeedSafe())
return false;
if (instructions_ && !instructions_->IsDontNeedSafe())
return false;
return true;
}

bool DartSnapshot::IsNullSafetyEnabled(const fml::Mapping* kernel) const {
return ::Dart_DetectNullSafety(
nullptr, // script_uri (unsupported by Flutter)
Expand Down
5 changes: 5 additions & 0 deletions runtime/dart_snapshot.h
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,11 @@ class DartSnapshot : public fml::RefCountedThreadSafe<DartSnapshot> {
///
const uint8_t* GetInstructionsMapping() const;

//----------------------------------------------------------------------------
/// @brief Returns whether both the data and instructions mappings are
/// safe to use with madvise(DONTNEED).
bool IsDontNeedSafe() const;

bool IsNullSafetyEnabled(
const fml::Mapping* application_kernel_mapping) const;

Expand Down
2 changes: 2 additions & 0 deletions shell/platform/android/apk_asset_provider.cc
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,8 @@ class APKAssetMapping : public fml::Mapping {
return reinterpret_cast<const uint8_t*>(AAsset_getBuffer(asset_));
}

bool IsDontNeedSafe() const override { return !AAsset_isAllocated(asset_); }

private:
AAsset* const asset_;

Expand Down
2 changes: 2 additions & 0 deletions shell/platform/darwin/common/buffer_conversions.mm
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@
return static_cast<const uint8_t*>([data_.get() bytes]);
}

bool IsDontNeedSafe() const override { return false; }

private:
fml::scoped_nsobject<NSData> data_;
FML_DISALLOW_COPY_AND_ASSIGN(NSDataMapping);
Expand Down
23 changes: 12 additions & 11 deletions shell/platform/fuchsia/flutter/component_v1.cc
Original file line number Diff line number Diff line change
Expand Up @@ -292,27 +292,28 @@ ComponentV1::ComponentV1(
}
auto hold_snapshot = [snapshot](const uint8_t* _, size_t __) {};
settings_.vm_snapshot_data = [hold_snapshot, vm_data]() {
return std::make_unique<fml::NonOwnedMapping>(vm_data, 0,
hold_snapshot);
return std::make_unique<fml::NonOwnedMapping>(vm_data, 0, hold_snapshot,
true /* dontneed_safe */);
};
settings_.vm_snapshot_instr = [hold_snapshot, vm_instructions]() {
return std::make_unique<fml::NonOwnedMapping>(vm_instructions, 0,
hold_snapshot);
return std::make_unique<fml::NonOwnedMapping>(
vm_instructions, 0, hold_snapshot, true /* dontneed_safe */);
};
settings_.isolate_snapshot_data = [hold_snapshot, isolate_data]() {
return std::make_unique<fml::NonOwnedMapping>(isolate_data, 0,
hold_snapshot);
return std::make_unique<fml::NonOwnedMapping>(
isolate_data, 0, hold_snapshot, true /* dontneed_safe */);
};
settings_.isolate_snapshot_instr = [hold_snapshot,
isolate_instructions]() {
return std::make_unique<fml::NonOwnedMapping>(isolate_instructions, 0,
hold_snapshot);
return std::make_unique<fml::NonOwnedMapping>(
isolate_instructions, 0, hold_snapshot, true /* dontneed_safe */);
};
isolate_snapshot_ = fml::MakeRefCounted<flutter::DartSnapshot>(
std::make_shared<fml::NonOwnedMapping>(isolate_data, 0,
hold_snapshot),
std::make_shared<fml::NonOwnedMapping>(isolate_data, 0, hold_snapshot,
true /* dontneed_safe */),
std::make_shared<fml::NonOwnedMapping>(isolate_instructions, 0,
hold_snapshot));
hold_snapshot,
true /* dontneed_safe */));
} else {
const int namespace_fd = component_data_directory_.get();
settings_.vm_snapshot_data = [namespace_fd]() {
Expand Down
Loading