Commit 09ef5458 authored by Jakob Gruber's avatar Jakob Gruber Committed by Commit Bot

[factory] Refactor JSFunction construction

Construction of JSFunction objects is complex, mostly due to the
existence of multiple functions kinds (JS, wasm, builtin, test, ...)
that are all created slightly differently. For example, JS functions
may come with an existing FeedbackCell (and FeedbackVector), while
builtins and wasm functions always use the many_closures_cell (without
a vector).

Prior to this CL, construction logic was scattered over a family of
7 functions, without a clearly defined chokepoint for header
initialization. This was hard to understand, hard to modify, and
needlessly inefficient (by setting some fields twice).

This CL fixes all that by introducing JSFunctionBuilder. The BuildRaw
method is the chokepoint for allocation and initialization, and Build
performs common pre- and post-work.

Future work:
- Remove now-deprecated functions.
- Untangle SFI/Map/JSFunction construction and remove
  Factory::NewFunction and NewFunctionArgs.

Bug: v8:8888
Change-Id: I709a2a44ee02e10593a4c9afe43d4d2c6d6351c4
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2527098Reviewed-by: 's avatarUlan Degenbaev <ulan@chromium.org>
Commit-Queue: Jakob Gruber <jgruber@chromium.org>
Cr-Commit-Position: refs/heads/master@{#71084}
parent 6b5d6d10
This diff is collapsed.
......@@ -631,11 +631,6 @@ class V8_EXPORT_PRIVATE Factory : public FactoryBase<Factory> {
// Function creation from SharedFunctionInfo.
Handle<JSFunction> NewFunctionFromSharedFunctionInfo(
Handle<Map> initial_map, Handle<SharedFunctionInfo> function_info,
Handle<Context> context, Handle<FeedbackCell> feedback_cell,
AllocationType allocation = AllocationType::kOld);
Handle<JSFunction> NewFunctionFromSharedFunctionInfo(
Handle<SharedFunctionInfo> function_info, Handle<Context> context,
Handle<FeedbackCell> feedback_cell,
......@@ -810,6 +805,43 @@ class V8_EXPORT_PRIVATE Factory : public FactoryBase<Factory> {
return New(map, allocation);
}
// Helper class for creating JSFunction objects.
class JSFunctionBuilder final {
public:
JSFunctionBuilder(Isolate* isolate, Handle<SharedFunctionInfo> sfi,
Handle<Context> context);
V8_WARN_UNUSED_RESULT Handle<JSFunction> Build();
JSFunctionBuilder& set_map(Handle<Map> v) {
maybe_map_ = v;
return *this;
}
JSFunctionBuilder& set_allocation_type(AllocationType v) {
allocation_type_ = v;
return *this;
}
JSFunctionBuilder& set_feedback_cell(Handle<FeedbackCell> v) {
maybe_feedback_cell_ = v;
return *this;
}
private:
void PrepareMap();
void PrepareFeedbackCell();
V8_WARN_UNUSED_RESULT Handle<JSFunction> BuildRaw(Handle<Code> code);
Isolate* const isolate_;
Handle<SharedFunctionInfo> sfi_;
Handle<Context> context_;
MaybeHandle<Map> maybe_map_;
MaybeHandle<FeedbackCell> maybe_feedback_cell_;
AllocationType allocation_type_ = AllocationType::kOld;
friend class Factory;
};
// Allows creation of Code objects. It provides two build methods, one of
// which tries to gracefully handle allocation failure.
class V8_EXPORT_PRIVATE CodeBuilder final {
......
......@@ -57,6 +57,17 @@ void FeedbackCell::SetInterruptBudget() {
set_interrupt_budget(FLAG_interrupt_budget);
}
void FeedbackCell::IncrementClosureCount(Isolate* isolate) {
ReadOnlyRoots r(isolate);
if (map() == r.no_closures_cell_map()) {
set_map(r.one_closure_cell_map());
} else if (map() == r.one_closure_cell_map()) {
set_map(r.many_closures_cell_map());
} else {
DCHECK(map() == r.many_closures_cell_map());
}
}
} // namespace internal
} // namespace v8
......
......@@ -36,6 +36,11 @@ class FeedbackCell : public TorqueGeneratedFeedbackCell<FeedbackCell, Struct> {
inline void SetInitialInterruptBudget();
inline void SetInterruptBudget();
// The closure count is encoded in the cell's map, which distinguishes
// between zero, one, or many closures. This function records a new closure
// creation by updating the map.
inline void IncrementClosureCount(Isolate* isolate);
using BodyDescriptor =
FixedBodyDescriptor<kValueOffset, kInterruptBudgetOffset, kAlignedSize>;
......
......@@ -434,8 +434,9 @@ void JSFunction::SetPrototype(Handle<JSFunction> function,
void JSFunction::SetInitialMap(Handle<JSFunction> function, Handle<Map> map,
Handle<HeapObject> prototype) {
if (map->prototype() != *prototype)
if (map->prototype() != *prototype) {
Map::SetPrototype(function->GetIsolate(), map, prototype);
}
function->set_prototype_or_initial_map(*map);
map->SetConstructor(*function);
if (FLAG_trace_maps) {
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment