Skip to content

Commit 804a306

Browse files
alexmarkovCommit Queue
alexmarkov
authored and
Commit Queue
committed
[vm] Preserve pragma annotations on hot reload
TEST=vm/cc/IsolateReload_KeepPragma{1,2,3} Change-Id: Ib698b918a27382e3573b5d3f0670d9f4e13c8064 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/341100 Commit-Queue: Alexander Markov <[email protected]> Reviewed-by: Ryan Macnak <[email protected]>
1 parent a3b7c32 commit 804a306

File tree

5 files changed

+233
-2
lines changed

5 files changed

+233
-2
lines changed

‎runtime/vm/canonical_tables.cc

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,8 +24,14 @@ bool MetadataMapTraits::IsMatch(const Object& a, const Object& b) {
2424
const String& name_b = String::Handle(Class::Cast(b).Name());
2525
return name_a.Equals(name_b);
2626
} else if (a.IsFunction() && b.IsFunction()) {
27-
const String& name_a = String::Handle(Function::Cast(a).name());
28-
const String& name_b = String::Handle(Function::Cast(b).name());
27+
const auto& func_a = Function::Cast(a);
28+
const auto& func_b = Function::Cast(b);
29+
if (func_a.IsNonImplicitClosureFunction() ||
30+
func_b.IsNonImplicitClosureFunction()) {
31+
return a.ptr() == b.ptr();
32+
}
33+
const String& name_a = String::Handle(func_a.name());
34+
const String& name_b = String::Handle(func_b.name());
2935
if (!name_a.Equals(name_b)) {
3036
return false;
3137
}

‎runtime/vm/isolate_reload.cc

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1565,6 +1565,8 @@ void ProgramReloadContext::CheckpointLibraries() {
15651565
// Add old library to old libraries set.
15661566
bool already_present = old_libraries_set.Insert(lib);
15671567
ASSERT(!already_present);
1568+
1569+
lib.EvaluatePragmas();
15681570
}
15691571
old_libraries_set_storage_ = old_libraries_set.Release().ptr();
15701572

@@ -1667,6 +1669,7 @@ void ProgramReloadContext::CommitBeforeInstanceMorphing() {
16671669
new_lib.set_native_entry_symbol_resolver(
16681670
lib.native_entry_symbol_resolver());
16691671
new_lib.set_ffi_native_resolver(lib.ffi_native_resolver());
1672+
new_lib.CopyPragmas(lib);
16701673
}
16711674
}
16721675

‎runtime/vm/isolate_reload_test.cc

Lines changed: 158 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6198,6 +6198,164 @@ TEST_CASE(IsolateReload_EnumInMainLibraryModified) {
61986198
EXPECT_STREQ("foo", SimpleInvokeStr(lib, "main"));
61996199
}
62006200

6201+
TEST_CASE(IsolateReload_KeepPragma1) {
6202+
// Old version of closure function bar() has a pragma.
6203+
const char* kScript =
6204+
"import 'file:///test:isolate_reload_helper';\n"
6205+
"foo() {\n"
6206+
" @pragma('vm:prefer-inline')\n"
6207+
" void bar() {}\n"
6208+
" return bar;\n"
6209+
"}"
6210+
"main() {\n"
6211+
" reloadTest();\n"
6212+
"}\n";
6213+
6214+
Dart_Handle lib = TestCase::LoadTestScript(kScript, nullptr);
6215+
EXPECT_VALID(lib);
6216+
6217+
// New version of closure function bar() doesn't have a pragma.
6218+
const char* kReloadScript =
6219+
"import 'file:///test:isolate_reload_helper';\n"
6220+
"foo() {\n"
6221+
" void bar() {}\n"
6222+
" return bar;\n"
6223+
"}"
6224+
"main() {\n"
6225+
" reloadTest();\n"
6226+
"}\n";
6227+
6228+
EXPECT_VALID(TestCase::SetReloadTestScript(kReloadScript));
6229+
6230+
Dart_Handle foo1_result = Dart_Invoke(lib, NewString("foo"), 0, nullptr);
6231+
EXPECT_VALID(foo1_result);
6232+
6233+
EXPECT_VALID(Dart_Invoke(lib, NewString("main"), 0, nullptr));
6234+
6235+
Dart_Handle foo2_result = Dart_Invoke(lib, NewString("foo"), 0, nullptr);
6236+
EXPECT_VALID(foo2_result);
6237+
6238+
TransitionNativeToVM transition(thread);
6239+
const auto& bar1 = Function::Handle(
6240+
Closure::Handle(Closure::RawCast(Api::UnwrapHandle(foo1_result)))
6241+
.function());
6242+
const auto& bar2 = Function::Handle(
6243+
Closure::Handle(Closure::RawCast(Api::UnwrapHandle(foo2_result)))
6244+
.function());
6245+
// Pragma should be retained on the old function,
6246+
// and should not appear on the new function.
6247+
EXPECT(Library::FindPragma(thread, /*only_core=*/false, bar1,
6248+
Symbols::vm_prefer_inline()));
6249+
EXPECT(!Library::FindPragma(thread, /*only_core=*/false, bar2,
6250+
Symbols::vm_prefer_inline()));
6251+
}
6252+
6253+
TEST_CASE(IsolateReload_KeepPragma2) {
6254+
// Old version of closure function bar() has a pragma.
6255+
const char* kScript =
6256+
"import 'file:///test:isolate_reload_helper';\n"
6257+
"foo() {\n"
6258+
" @pragma('vm:prefer-inline')\n"
6259+
" void bar() {}\n"
6260+
" return bar;\n"
6261+
"}"
6262+
"main() {\n"
6263+
" reloadTest();\n"
6264+
"}\n";
6265+
6266+
Dart_Handle lib = TestCase::LoadTestScript(kScript, nullptr);
6267+
EXPECT_VALID(lib);
6268+
6269+
// New version of closure function bar() has a different pragma.
6270+
const char* kReloadScript =
6271+
"import 'file:///test:isolate_reload_helper';\n"
6272+
"foo() {\n"
6273+
" @pragma('vm:never-inline')\n"
6274+
" void bar() {}\n"
6275+
" return bar;\n"
6276+
"}"
6277+
"main() {\n"
6278+
" reloadTest();\n"
6279+
"}\n";
6280+
6281+
EXPECT_VALID(TestCase::SetReloadTestScript(kReloadScript));
6282+
6283+
Dart_Handle foo1_result = Dart_Invoke(lib, NewString("foo"), 0, nullptr);
6284+
EXPECT_VALID(foo1_result);
6285+
6286+
EXPECT_VALID(Dart_Invoke(lib, NewString("main"), 0, nullptr));
6287+
6288+
Dart_Handle foo2_result = Dart_Invoke(lib, NewString("foo"), 0, nullptr);
6289+
EXPECT_VALID(foo2_result);
6290+
6291+
TransitionNativeToVM transition(thread);
6292+
const auto& bar1 = Function::Handle(
6293+
Closure::Handle(Closure::RawCast(Api::UnwrapHandle(foo1_result)))
6294+
.function());
6295+
const auto& bar2 = Function::Handle(
6296+
Closure::Handle(Closure::RawCast(Api::UnwrapHandle(foo2_result)))
6297+
.function());
6298+
EXPECT(Library::FindPragma(thread, /*only_core=*/false, bar1,
6299+
Symbols::vm_prefer_inline()));
6300+
EXPECT(!Library::FindPragma(thread, /*only_core=*/false, bar1,
6301+
Symbols::vm_never_inline()));
6302+
EXPECT(!Library::FindPragma(thread, /*only_core=*/false, bar2,
6303+
Symbols::vm_prefer_inline()));
6304+
EXPECT(Library::FindPragma(thread, /*only_core=*/false, bar2,
6305+
Symbols::vm_never_inline()));
6306+
}
6307+
6308+
TEST_CASE(IsolateReload_KeepPragma3) {
6309+
// Old version of closure function bar() doesn't have a pragma.
6310+
const char* kScript =
6311+
"import 'file:///test:isolate_reload_helper';\n"
6312+
"foo() {\n"
6313+
" void bar() {}\n"
6314+
" return bar;\n"
6315+
"}"
6316+
"main() {\n"
6317+
" reloadTest();\n"
6318+
"}\n";
6319+
6320+
Dart_Handle lib = TestCase::LoadTestScript(kScript, nullptr);
6321+
EXPECT_VALID(lib);
6322+
6323+
// New version of closure function bar() has a pragma.
6324+
const char* kReloadScript =
6325+
"import 'file:///test:isolate_reload_helper';\n"
6326+
"foo() {\n"
6327+
" @pragma('vm:never-inline')\n"
6328+
" void bar() {}\n"
6329+
" return bar;\n"
6330+
"}"
6331+
"main() {\n"
6332+
" reloadTest();\n"
6333+
"}\n";
6334+
6335+
EXPECT_VALID(TestCase::SetReloadTestScript(kReloadScript));
6336+
6337+
Dart_Handle foo1_result = Dart_Invoke(lib, NewString("foo"), 0, nullptr);
6338+
EXPECT_VALID(foo1_result);
6339+
6340+
EXPECT_VALID(Dart_Invoke(lib, NewString("main"), 0, nullptr));
6341+
6342+
Dart_Handle foo2_result = Dart_Invoke(lib, NewString("foo"), 0, nullptr);
6343+
EXPECT_VALID(foo2_result);
6344+
6345+
TransitionNativeToVM transition(thread);
6346+
const auto& bar1 = Function::Handle(
6347+
Closure::Handle(Closure::RawCast(Api::UnwrapHandle(foo1_result)))
6348+
.function());
6349+
const auto& bar2 = Function::Handle(
6350+
Closure::Handle(Closure::RawCast(Api::UnwrapHandle(foo2_result)))
6351+
.function());
6352+
EXPECT(Library::FindPragma(thread, /*only_core=*/false, bar2,
6353+
Symbols::vm_never_inline()));
6354+
// Should not appear on previous version of bar().
6355+
EXPECT(!Library::FindPragma(thread, /*only_core=*/false, bar1,
6356+
Symbols::vm_never_inline()));
6357+
}
6358+
62016359
#endif // !defined(PRODUCT) && !defined(DART_PRECOMPILED_RUNTIME)
62026360

62036361
} // namespace dart

‎runtime/vm/object.cc

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13770,6 +13770,65 @@ ObjectPtr Library::GetMetadata(const Object& declaration) const {
1377013770
#endif // defined(DART_PRECOMPILED_RUNTIME)
1377113771
}
1377213772

13773+
#if !defined(DART_PRECOMPILED_RUNTIME)
13774+
static bool HasPragma(const Object& declaration) {
13775+
return (declaration.IsClass() && Class::Cast(declaration).has_pragma()) ||
13776+
(declaration.IsFunction() &&
13777+
Function::Cast(declaration).has_pragma()) ||
13778+
(declaration.IsField() && Field::Cast(declaration).has_pragma());
13779+
}
13780+
13781+
void Library::EvaluatePragmas() {
13782+
Object& declaration = Object::Handle();
13783+
const GrowableObjectArray& declarations =
13784+
GrowableObjectArray::Handle(GrowableObjectArray::New());
13785+
{
13786+
auto thread = Thread::Current();
13787+
SafepointReadRwLocker ml(thread, thread->isolate_group()->program_lock());
13788+
MetadataMap map(metadata());
13789+
MetadataMap::Iterator it(&map);
13790+
while (it.MoveNext()) {
13791+
const intptr_t entry = it.Current();
13792+
ASSERT(entry != -1);
13793+
declaration = map.GetKey(entry);
13794+
if (HasPragma(declaration)) {
13795+
declarations.Add(declaration);
13796+
}
13797+
}
13798+
set_metadata(map.Release());
13799+
}
13800+
for (intptr_t i = 0; i < declarations.Length(); ++i) {
13801+
declaration = declarations.At(i);
13802+
GetMetadata(declaration);
13803+
}
13804+
}
13805+
13806+
void Library::CopyPragmas(const Library& old_lib) {
13807+
auto thread = Thread::Current();
13808+
SafepointWriteRwLocker ml(thread, thread->isolate_group()->program_lock());
13809+
MetadataMap new_map(metadata());
13810+
MetadataMap old_map(old_lib.metadata());
13811+
Object& declaration = Object::Handle();
13812+
Object& value = Object::Handle();
13813+
MetadataMap::Iterator it(&old_map);
13814+
while (it.MoveNext()) {
13815+
const intptr_t entry = it.Current();
13816+
ASSERT(entry != -1);
13817+
declaration = old_map.GetKey(entry);
13818+
if (HasPragma(declaration)) {
13819+
value = old_map.GetPayload(entry, 0);
13820+
ASSERT(!value.IsNull());
13821+
// Pragmas should be evaluated during hot reload phase 1
13822+
// (when checkpointing libraries).
13823+
ASSERT(!value.IsSmi());
13824+
new_map.UpdateOrInsert(declaration, value);
13825+
}
13826+
}
13827+
old_lib.set_metadata(old_map.Release());
13828+
set_metadata(new_map.Release());
13829+
}
13830+
#endif // !defined(DART_PRECOMPILED_RUNTIME)
13831+
1377313832
static bool ShouldBePrivate(const String& name) {
1377413833
return (name.Length() >= 1 && name.CharAt(0) == '_') ||
1377513834
(name.Length() >= 5 &&

‎runtime/vm/object.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5136,6 +5136,11 @@ class Library : public Object {
51365136
void AddMetadata(const Object& declaration, intptr_t kernel_offset) const;
51375137
ObjectPtr GetMetadata(const Object& declaration) const;
51385138

5139+
#if !defined(DART_PRECOMPILED_RUNTIME)
5140+
void EvaluatePragmas();
5141+
void CopyPragmas(const Library& old_lib);
5142+
#endif
5143+
51395144
// Tries to finds a @pragma annotation on [object].
51405145
//
51415146
// If successful returns `true`. If an error happens during constant

0 commit comments

Comments
 (0)