Skip to content

Commit 6052a0a

Browse files
committed
[runtime] sort transition array after it grows beyond linear search size
Previously TransitionsAccessor::InsertHelper() can grow the transition array without making sure it's sorted after the size crosses kMaxElementsForLinearSearch, leading to search failures or duplicate entries in bigger transition arrays. See nodejs/node#61002 (comment) Change-Id: I3f45816b7d65bbbcfedeb7074d0c6907cbc08edf
1 parent 4e39687 commit 6052a0a

2 files changed

Lines changed: 45 additions & 0 deletions

File tree

src/objects/transitions.cc

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -173,6 +173,11 @@ void TransitionsAccessor::InsertHelper(Isolate* isolate, DirectHandle<Map> map,
173173
}
174174
array->SetKey(insertion_index, *name);
175175
array->SetRawTarget(insertion_index, MakeWeak(*target));
176+
// The new size exceeds the threshold for linear search, sort the array
177+
// for binary search later.
178+
// if (new_nof == TransitionArray::kMaxElementsForLinearSearch + 1) {
179+
// array->Sort();
180+
// }
176181
SLOW_DCHECK(array->IsSortedNoDuplicates());
177182
return;
178183
}
@@ -222,6 +227,11 @@ void TransitionsAccessor::InsertHelper(Isolate* isolate, DirectHandle<Map> map,
222227
result->Set(i + 1, array->GetKey(i), array->GetRawTarget(i));
223228
}
224229

230+
// The new size exceeds the threshold for linear search, sort the array
231+
// for binary search later.
232+
// if (new_nof == TransitionArray::kMaxElementsForLinearSearch + 1) {
233+
// result->Sort();
234+
// }
225235
SLOW_DCHECK(result->IsSortedNoDuplicates());
226236
ReplaceTransitions(isolate, map, result);
227237
}

test/cctest/test-transitions.cc

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88

99
#include <utility>
1010

11+
#include "src/api/api-inl.h"
1112
#include "src/codegen/compilation-cache.h"
1213
#include "src/execution/execution.h"
1314
#include "src/heap/factory.h"
@@ -316,5 +317,39 @@ TEST(TransitionArray_SameFieldNamesDifferentAttributes) {
316317
DCHECK(transitions.IsSortedNoDuplicates());
317318
}
318319

320+
TEST(TransitionArray_InsertionAfterLinearThreshold) {
321+
CcTest::InitializeVM();
322+
v8::Isolate* isolate = CcTest::isolate();
323+
Isolate* i_isolate = CcTest::i_isolate();
324+
v8::HandleScope scope(isolate);
325+
326+
// A shuffled permuatation of the characters so that we can insert transitions
327+
// and check that they are sorted after ht
328+
std::string names = "HNkgAQeqBsnyCDEOiPFRUxfwcIKtlVZTMYbdvJpLhXmouzrGajSW";
329+
std::reverse(names.begin(), names.end());
330+
CHECK_GT(names.size(), TransitionArray::kMaxElementsForLinearSearch);
331+
v8::Local<v8::Context> context = isolate->GetCurrentContext();
332+
DirectHandle<Map> first_map;
333+
int len = static_cast<int>(names.size());
334+
for (int i = 0; i < len; i++) {
335+
v8::Local<v8::Object> obj = v8::Object::New(isolate);
336+
if (i == 0) {
337+
first_map =
338+
direct_handle(v8::Utils::OpenDirectHandle(*obj)->map(), i_isolate);
339+
}
340+
v8::Local<v8::String> name =
341+
v8::String::NewFromOneByte(isolate,
342+
reinterpret_cast<const uint8_t*>(&names[i]),
343+
v8::NewStringType::kNormal, 1)
344+
.ToLocalChecked();
345+
obj->Set(context, name, v8::Integer::New(isolate, i)).Check();
346+
DirectHandle<Map> current_map(v8::Utils::OpenDirectHandle(*obj)->map(),
347+
i_isolate);
348+
}
349+
TestTransitionsAccessor transitions(i_isolate, first_map);
350+
CHECK_EQ(names.size(), transitions.NumberOfTransitions());
351+
CHECK(transitions.transitions()->IsSortedNoDuplicates());
352+
}
353+
319354
} // namespace internal
320355
} // namespace v8

0 commit comments

Comments
 (0)