Skip to content

Commit

Permalink
Skip tracking users for literals
Browse files Browse the repository at this point in the history
Summary:
The user lists for literals can grow extremely large since they are
shared across the module, and may occur many times in object and array
literals. Updating usage of literals can be very costly, since it needs
to iterate the user list to remove the prior use.

One example where this is currently very slow is when deleting
AllocObjectLiteral when it gets lowered:

1. Iterate over the (potentially large) list of operands of the
instruction to remove each one.
2. For each operand, iterate the user list of the corresponding literal
to find its use.
3. Once the use is found and removed, the last user gets moved into its
position. This requires updating the corresponding `Use`, which means
iterating through the operands of the using instruction.

Reviewed By: avp

Differential Revision: D67290808

fbshipit-source-id: 9344fecc5d56ebdeab25a821c00dc299aed46cda
  • Loading branch information
neildhar authored and facebook-github-bot committed Feb 1, 2025
1 parent f807f90 commit 1d7433b
Show file tree
Hide file tree
Showing 5 changed files with 52 additions and 8 deletions.
9 changes: 9 additions & 0 deletions include/hermes/IR/IR.h
Original file line number Diff line number Diff line change
Expand Up @@ -717,6 +717,9 @@ class Value {
/// Run a Value's destructor and deallocate its memory.
static void destroy(Value *V);

/// \return true if this instruction tracks its users.
inline bool tracksUsers() const;

/// \return the users of the value.
const UseListTy &getUsers() const;

Expand Down Expand Up @@ -2682,6 +2685,12 @@ static inline llvh::hash_code hash_value(Type V) {
return V.hash();
}

inline bool Value::tracksUsers() const {
// We do not track users of literals because we never need to enumerate them
// and they can be very costly to maintain.
return !llvh::isa<Literal>(this);
}

inline Function *Instruction::getFunction() const {
return Parent->getParent();
}
Expand Down
15 changes: 14 additions & 1 deletion lib/IR/IR.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -74,24 +74,32 @@ llvh::StringRef Value::getKindStr() const {
}

const Value::UseListTy &Value::getUsers() const {
assert(tracksUsers() && "Instruction does not track its users.");
return Users;
}

unsigned Value::getNumUsers() const {
assert(tracksUsers() && "Instruction does not track its users.");
return Users.size();
}

bool Value::hasUsers() const {
assert(tracksUsers() && "Instruction does not track its users.");
return Users.size();
}

bool Value::hasOneUser() const {
assert(tracksUsers() && "Instruction does not track its users.");
return 1 == Users.size();
}

void Value::removeUse(Use U) {
assert(Users.size() && "Removing a user from an empty list");
assert(U.first == this && "Invalid user");
// If the instruction should not track users, do nothing.
if (!tracksUsers())
return;

assert(Users.size() && "Removing a user from an empty list");

// We don't care about the order of the operands in the use vector. One cheap
// way to delete an element is to pop the last element and save it on top of
Expand All @@ -116,11 +124,16 @@ void Value::removeUse(Use U) {
}

Value::Use Value::addUser(Instruction *Inst) {
// If the instruction should not track users, just return a dummy use.
if (!tracksUsers())
return {this, 0};

Users.push_back(Inst);
return {this, static_cast<unsigned>(Users.size() - 1)};
}

void Value::replaceAllUsesWith(Value *Other) {
assert(tracksUsers() && "Instruction does not track its users.");
if (this == Other) {
return;
}
Expand Down
10 changes: 6 additions & 4 deletions lib/IR/IRVerifier.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -548,10 +548,12 @@ bool Verifier::verifyBeforeVisitInstruction(const Instruction &Inst) {
for (unsigned i = 0; i < Inst.getNumOperands(); i++) {
auto Operand = Inst.getOperand(i);
AssertIWithMsg(Inst, Operand != nullptr, "Invalid operand");
AssertIWithMsg(
Inst,
getUsersSetForValue(Operand).count(&Inst) == 1,
"This instruction is not in the User list of the operand");
if (Operand->tracksUsers()) {
AssertIWithMsg(
Inst,
getUsersSetForValue(Operand).count(&Inst) == 1,
"This instruction is not in the User list of the operand");
}
if (llvh::isa<Variable>(Operand)) {
AssertIWithMsg(
Inst,
Expand Down
6 changes: 3 additions & 3 deletions lib/Optimizer/Scalar/TDZDedup.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -176,9 +176,9 @@ bool TDZDedupContext::processNode(StackNode *SN) {
// If ThrowIf has no users, we will attempt to destroy the load too, to
// save work in other passes.
if (!TIE->hasUsers()) {
if (TIE->getCheckedValue()->hasOneUser() &&
(llvh::isa<LoadFrameInst>(TIE->getCheckedValue()) ||
llvh::isa<LoadStackInst>(TIE->getCheckedValue()))) {
if ((llvh::isa<LoadFrameInst>(TIE->getCheckedValue()) ||
llvh::isa<LoadStackInst>(TIE->getCheckedValue())) &&
TIE->getCheckedValue()->hasOneUser()) {
destroyer.add(llvh::cast<Instruction>(TIE->getCheckedValue()));
}
} else {
Expand Down
20 changes: 20 additions & 0 deletions test/hermes/literal_obj_prop_limit.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
/**
* Copyright (c) Meta Platforms, Inc. and affiliates.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*/

// RUN: %hermes %s | (! %hermes - 2>&1 -gc-sanitize-handles=0 ) | %FileCheck --match-full-lines %s
// REQUIRES: !slow_debug

// Print a huge object literal with 300,000 entries.

print("globalThis.obj = {");

for(let i = 0; i < 300_000; ++i)
print ("a" + i, ": null,");

print("};")

// CHECK: Uncaught RangeError: Object has more than {{.+}} properties

0 comments on commit 1d7433b

Please sign in to comment.