Skip to content

Commit

Permalink
[JSC] Separate JSString::destroy and JSRopeString::destroy
Browse files Browse the repository at this point in the history
https://bugs.webkit.org/show_bug.cgi?id=259388
rdar://112648672

Reviewed by Keith Miller.

We would like to separate them so that,

1. We can easily collect each sample from profiler.
2. We do not need to have isRope check for JSString case.

* Source/JavaScriptCore/heap/Heap.h:
* Source/JavaScriptCore/runtime/JSString.h:
(JSC::JSString::destroy): Deleted.
* Source/JavaScriptCore/runtime/JSStringInlines.h:
(JSC::JSString::destroy):
(JSC::JSRopeString::destroy):
(JSC::JSString::~JSString): Deleted.

Canonical link: https://commits.webkit.org/266206@main
  • Loading branch information
Constellation committed Jul 21, 2023
1 parent d16e5b7 commit 54067b8
Show file tree
Hide file tree
Showing 3 changed files with 18 additions and 12 deletions.
4 changes: 3 additions & 1 deletion Source/JavaScriptCore/heap/Heap.h
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ class JITStubRoutine;
class JITStubRoutineSet;
class JSCell;
class JSImmutableButterfly;
class JSRopeString;
class JSString;
class JSValue;
class MachineThreads;
Expand Down Expand Up @@ -133,7 +134,7 @@ class Heap;
v(propertyTableSpace, destructibleCellHeapCellType, PropertyTable) \
v(regExpSpace, destructibleCellHeapCellType, RegExp) \
v(regExpObjectSpace, cellHeapCellType, RegExpObject) \
v(ropeStringSpace, stringHeapCellType, JSRopeString) \
v(ropeStringSpace, ropeStringHeapCellType, JSRopeString) \
v(scopedArgumentsSpace, cellHeapCellType, ScopedArguments) \
v(sparseArrayValueMapSpace, destructibleCellHeapCellType, SparseArrayValueMap) \
v(stringSpace, stringHeapCellType, JSString) \
Expand Down Expand Up @@ -943,6 +944,7 @@ class Heap {
IsoHeapCellType moduleNamespaceObjectHeapCellType;
IsoHeapCellType nativeStdFunctionHeapCellType;
IsoInlinedHeapCellType<JSString> stringHeapCellType;
IsoInlinedHeapCellType<JSRopeString> ropeStringHeapCellType;
IsoHeapCellType weakMapHeapCellType;
IsoHeapCellType weakSetHeapCellType;
JSDestructibleObjectHeapCellType destructibleObjectHeapCellType;
Expand Down
11 changes: 4 additions & 7 deletions Source/JavaScriptCore/runtime/JSString.h
Original file line number Diff line number Diff line change
Expand Up @@ -110,11 +110,8 @@ class JSString : public JSCell {
static constexpr unsigned StructureFlags = Base::StructureFlags | OverridesGetOwnPropertySlot | InterceptsGetOwnPropertySlotByIndexEvenWhenLengthIsNotZero | StructureIsImmortal | OverridesPut;

static constexpr bool needsDestruction = true;
static ALWAYS_INLINE void destroy(JSCell* cell)
{
static_cast<JSString*>(cell)->JSString::~JSString();
}

static void destroy(JSCell*);

// We specialize the string subspace to get the fastest possible sweep. This wouldn't be
// necessary if JSString didn't have a destructor.
template<typename, SubspaceAccess>
Expand Down Expand Up @@ -201,8 +198,6 @@ class JSString : public JSCell {
DECLARE_DEFAULT_FINISH_CREATION;

public:
~JSString();

Identifier toIdentifier(JSGlobalObject*) const;
AtomString toAtomString(JSGlobalObject*) const;
AtomString toExistingAtomString(JSGlobalObject*) const;
Expand Down Expand Up @@ -292,6 +287,8 @@ class JSString : public JSCell {
class JSRopeString final : public JSString {
friend class JSString;
public:
static void destroy(JSCell*);

template<typename, SubspaceAccess>
static GCClient::IsoSubspace* subspaceFor(VM& vm)
{
Expand Down
15 changes: 11 additions & 4 deletions Source/JavaScriptCore/runtime/JSStringInlines.h
Original file line number Diff line number Diff line change
Expand Up @@ -30,12 +30,19 @@
#include "KeyAtomStringCacheInlines.h"

namespace JSC {

inline JSString::~JSString()

ALWAYS_INLINE void JSString::destroy(JSCell* cell)
{
auto* string = static_cast<JSString*>(cell);
string->valueInternal().~String();
}

ALWAYS_INLINE void JSRopeString::destroy(JSCell* cell)
{
if (isRope())
auto* string = static_cast<JSRopeString*>(cell);
if (string->isRope())
return;
valueInternal().~String();
string->valueInternal().~String();
}

bool JSString::equal(JSGlobalObject* globalObject, JSString* other) const
Expand Down

0 comments on commit 54067b8

Please sign in to comment.