Attachment #238431 for bug #136983

View Details Raw Unified Return to Bug 136983

| Differences between



2014-09-21  Filip Pizlo  <fpizlo@apple.com>

        Structure should have a method for concurrently getting all of the property map entries, and this method shouldn't involve copy-paste
        https://bugs.webkit.org/show_bug.cgi?id=136983

        Reviewed by NOBODY (OOPS!).

        * runtime/PropertyMapHashTable.h:
        (JSC::PropertyMapEntry::PropertyMapEntry): Moved PropertyMapEntry struct to Structure.h so that Structure can refer to it.
        * runtime/Structure.cpp:
        (JSC::Structure::getConcurrently): Switch to using the new forEachPropertyConcurrently() method.
        (JSC::Structure::getPropertiesConcurrently): The subject of this patch. It will be useful for object allocation sinking (bug 136330).
        (JSC::Structure::dump): Switch to using the new forEachPropertyConcurrently() method.
        * runtime/Structure.h:
        (JSC::PropertyMapEntry::PropertyMapEntry): Moved from PropertyMapHashTable.h.
        * runtime/StructureInlines.h:
        (JSC::Structure::forEachPropertyConcurrently): Capture this very common concurrent structure iteration pattern into a template method.

2014-09-21  Filip Pizlo  <fpizlo@apple.com>

        Structure::getConcurrently() doesn't need to take a VM& argument.



    return v;
}

struct PropertyMapEntry {
    StringImpl* key;
    PropertyOffset offset;
    unsigned attributes;

    PropertyMapEntry(StringImpl* key, PropertyOffset offset, unsigned attributes)
        : key(key)
        , offset(offset)
        , attributes(attributes)
    {
    }
};

class PropertyTable : public JSCell {

    // This is the implementation for 'iterator' and 'const_iterator',




PropertyOffset Structure::getConcurrently(StringImpl* uid, unsigned& attributes)
{
    PropertyOffset result = invalidOffset;
    Structure* structure;
    PropertyTable* table;
    
    findStructuresAndMapForMaterialization(structures, structure, table);
    
    if (table) {
        PropertyMapEntry* entry = table->get(uid);
        if (entry) {
            attributes = entry->attributes;
            PropertyOffset result = entry->offset;
            structure->m_lock.unlock();
            return result;
        }
        structure->m_lock.unlock();
    }
    
    forEachPropertyConcurrently(
        [&] (const PropertyMapEntry& candidate) -> bool {
            if (candidate.key != uid)
                return true;
            
            result = candidate.offset;
            attributes = candidate.attributes;
            return false;
        });
    
    return result;
}

Vector<PropertyMapEntry> Structure::getPropertiesConcurrently()
{
    Vector<PropertyMapEntry> result;

    forEachPropertyConcurrently(
        [&] (const PropertyMapEntry& entry) -> bool {
            result.append(entry);
            return true;
        });
    
    return result;
}

PropertyOffset Structure::add(VM& vm, PropertyName propertyName, unsigned attributes)

{
    out.print(RawPointer(this), ":[", classInfo()->className, ", {");
    
    Vector<Structure*, 8> structures;
    Structure* structure;
    PropertyTable* table;
    
    const_cast<Structure*>(this)->findStructuresAndMapForMaterialization(
        structures, structure, table);
    
    CommaPrinter comma;
    
    const_cast<Structure*>(this)->forEachPropertyConcurrently(
        [&] (const PropertyMapEntry& entry) -> bool {
            out.print(comma, entry.key, ":", static_cast<int>(entry.offset));
            return true;
        });
        
        structure->m_lock.unlock();
    }
    
    for (unsigned i = structures.size(); i--;) {
        Structure* structure = structures[i];
        if (!structure->m_nameInPrevious)
            continue;
        out.print(comma, structure->m_nameInPrevious.get(), ":", static_cast<int>(structure->m_offset));
    }
    
    out.print("}, ", IndexingTypeDump(indexingType()));
    



// initial allocation.
static const unsigned outOfLineGrowthFactor = 2;

struct PropertyMapEntry {
    StringImpl* key;
    PropertyOffset offset;
    unsigned attributes;

    PropertyMapEntry()
        : key(nullptr)
        , offset(invalidOffset)
        , attributes(0)
    {
    }
    
    PropertyMapEntry(StringImpl* key, PropertyOffset offset, unsigned attributes)
        : key(key)
        , offset(offset)
        , attributes(attributes)
    {
    }
};

class Structure : public JSCell {
public:
    friend class StructureTransitionTable;

    PropertyOffset get(VM&, PropertyName);
    PropertyOffset get(VM&, PropertyName, unsigned& attributes);

    // This is a somewhat internalish method. It will call your functor while possibly holding the
    // Structure's lock. There is no guarantee whether the lock is held or not in any particular
    // call. So, you have to assume the worst. Also, the functor returns true if it wishes for you
    // to continue or false if it's done.
    template<typename Functor>
    void forEachPropertyConcurrently(const Functor&);
    
    PropertyOffset getConcurrently(StringImpl* uid);
    PropertyOffset getConcurrently(StringImpl* uid, unsigned& attributes);
    
    Vector<PropertyMapEntry> getPropertiesConcurrently();
    
    void setHasGetterSetterPropertiesWithProtoCheck(bool is__proto__)
    {
        setHasGetterSetterProperties(true);



/*
 * Copyright (C) 2013, 2014 Apple Inc. All rights reserved.
 *
 * Redistribution and use in source and binary forms, with or without
 * modification, are permitted provided that the following conditions

    return entry->offset;
}

template<typename Functor>
void Structure::forEachPropertyConcurrently(const Functor& functor)
{
    Vector<Structure*, 8> structures;
    Structure* structure;
    PropertyTable* table;
    
    findStructuresAndMapForMaterialization(structures, structure, table);
    
    if (table) {
        for (auto entry : *table) {
            if (!functor(entry)) {
                structure->m_lock.unlock();
                return;
            }
        }
        structure->m_lock.unlock();
    }
    
    for (unsigned i = structures.size(); i--;) {
        structure = structures[i];
        if (!structure->m_nameInPrevious)
            continue;
        
        if (!functor(PropertyMapEntry(structure->m_nameInPrevious.get(), structure->m_offset, structure->attributesInPrevious())))
            return;
    }
}

inline PropertyOffset Structure::getConcurrently(StringImpl* uid)
{
    unsigned attributesIgnored;

Return to Bug 136983

Lines 78-96 inline unsigned nextPowerOf2(unsigned v) Source/JavaScriptCore/runtime/PropertyMapHashTable.h_sec1
- Source/JavaScriptCore/runtime/PropertyMapHashTable.h -13 lines
78	return v;	78	return v;
79	}	79	}
80		80
81	struct PropertyMapEntry {
82	StringImpl* key;
83	PropertyOffset offset;
84	unsigned attributes;
85
86	PropertyMapEntry(StringImpl* key, PropertyOffset offset, unsigned attributes)
87	: key(key)
88	, offset(offset)
89	, attributes(attributes)
90	{
91	}
92	};
93
94	class PropertyTable : public JSCell {	81	class PropertyTable : public JSCell {
95		82
96	// This is the implementation for 'iterator' and 'const_iterator',	83	// This is the implementation for 'iterator' and 'const_iterator',

Lines 852-884 PropertyTable* Structure::copyPropertyTa Source/JavaScriptCore/runtime/Structure.cpp_sec1
- Source/JavaScriptCore/runtime/Structure.cpp -47 / +29 lines
852		852
853	PropertyOffset Structure::getConcurrently(StringImpl* uid, unsigned& attributes)	853	PropertyOffset Structure::getConcurrently(StringImpl* uid, unsigned& attributes)
854	{	854	{
855	Vector<Structure*, 8> structures;	855	PropertyOffset result = invalidOffset;
856	Structure* structure;
857	PropertyTable* table;
858
859	findStructuresAndMapForMaterialization(structures, structure, table);
860
861	if (table) {
862	PropertyMapEntry* entry = table->get(uid);
863	if (entry) {
864	attributes = entry->attributes;
865	PropertyOffset result = entry->offset;
866	structure->m_lock.unlock();
867	return result;
868	}
869	structure->m_lock.unlock();
870	}
871		856
872	for (unsigned i = structures.size(); i--;) {	857	forEachPropertyConcurrently(
873	structure = structures[i];	858	[&] (const PropertyMapEntry& candidate) -> bool {
874	if (structure->m_nameInPrevious.get() != uid)	859	if (candidate.key != uid)
875	continue;	860	return true;
876		861
877	attributes = structure->attributesInPrevious();	862	result = candidate.offset;
878	return structure->m_offset;	863	attributes = candidate.attributes;
879	}	864	return false;
		865	});
880		866
881	return invalidOffset;	867	return result;
		868	}
		869
		870	Vector<PropertyMapEntry> Structure::getPropertiesConcurrently()
		871	{
		872	Vector<PropertyMapEntry> result;
		873
		874	forEachPropertyConcurrently(
		875	[&] (const PropertyMapEntry& entry) -> bool {
		876	result.append(entry);
		877	return true;
		878	});
		879
		880	return result;
882	}	881	}
883		882
884	PropertyOffset Structure::add(VM& vm, PropertyName propertyName, unsigned attributes)	883	PropertyOffset Structure::add(VM& vm, PropertyName propertyName, unsigned attributes)
Lines 1110-1139 void Structure::dump(PrintStream& out) c Source/JavaScriptCore/runtime/Structure.cpp_sec2
1110	{	1109	{
1111	out.print(RawPointer(this), ":[", classInfo()->className, ", {");	1110	out.print(RawPointer(this), ":[", classInfo()->className, ", {");
1112		1111
1113	Vector<Structure*, 8> structures;
1114	Structure* structure;
1115	PropertyTable* table;
1116
1117	const_cast<Structure*>(this)->findStructuresAndMapForMaterialization(
1118	structures, structure, table);
1119
1120	CommaPrinter comma;	1112	CommaPrinter comma;
1121		1113
1122	if (table) {	1114	const_cast<Structure*>(this)->forEachPropertyConcurrently(
1123	PropertyTable::iterator iter = table->begin();	1115	[&] (const PropertyMapEntry& entry) -> bool {
1124	PropertyTable::iterator end = table->end();	1116	out.print(comma, entry.key, ":", static_cast<int>(entry.offset));
1125	for (; iter != end; ++iter)	1117	return true;
1126	out.print(comma, iter->key, ":", static_cast<int>(iter->offset));	1118	});
1127
1128	structure->m_lock.unlock();
1129	}
1130
1131	for (unsigned i = structures.size(); i--;) {
1132	Structure* structure = structures[i];
1133	if (!structure->m_nameInPrevious)
1134	continue;
1135	out.print(comma, structure->m_nameInPrevious.get(), ":", static_cast<int>(structure->m_offset));
1136	}
1137		1119
1138	out.print("}, ", IndexingTypeDump(indexingType()));	1120	out.print("}, ", IndexingTypeDump(indexingType()));
1139		1121

Lines 73-78 static const unsigned initialOutOfLineCa Source/JavaScriptCore/runtime/Structure.h_sec1
- Source/JavaScriptCore/runtime/Structure.h +29 lines
73	// initial allocation.	73	// initial allocation.
74	static const unsigned outOfLineGrowthFactor = 2;	74	static const unsigned outOfLineGrowthFactor = 2;
75		75
		76	struct PropertyMapEntry {
		77	StringImpl* key;
		78	PropertyOffset offset;
		79	unsigned attributes;
		80
		81	PropertyMapEntry()
		82	: key(nullptr)
		83	, offset(invalidOffset)
		84	, attributes(0)
		85	{
		86	}
		87
		88	PropertyMapEntry(StringImpl* key, PropertyOffset offset, unsigned attributes)
		89	: key(key)
		90	, offset(offset)
		91	, attributes(attributes)
		92	{
		93	}
		94	};
		95
76	class Structure : public JSCell {	96	class Structure : public JSCell {
77	public:	97	public:
78	friend class StructureTransitionTable;	98	friend class StructureTransitionTable;
Lines 262-270 public: Source/JavaScriptCore/runtime/Structure.h_sec2
262	PropertyOffset get(VM&, PropertyName);	282	PropertyOffset get(VM&, PropertyName);
263	PropertyOffset get(VM&, PropertyName, unsigned& attributes);	283	PropertyOffset get(VM&, PropertyName, unsigned& attributes);
264		284
		285	// This is a somewhat internalish method. It will call your functor while possibly holding the
		286	// Structure's lock. There is no guarantee whether the lock is held or not in any particular
		287	// call. So, you have to assume the worst. Also, the functor returns true if it wishes for you
		288	// to continue or false if it's done.
		289	template<typename Functor>
		290	void forEachPropertyConcurrently(const Functor&);
		291
265	PropertyOffset getConcurrently(StringImpl* uid);	292	PropertyOffset getConcurrently(StringImpl* uid);
266	PropertyOffset getConcurrently(StringImpl* uid, unsigned& attributes);	293	PropertyOffset getConcurrently(StringImpl* uid, unsigned& attributes);
267		294
		295	Vector<PropertyMapEntry> getPropertiesConcurrently();
		296
268	void setHasGetterSetterPropertiesWithProtoCheck(bool is__proto__)	297	void setHasGetterSetterPropertiesWithProtoCheck(bool is__proto__)
269	{	298	{
270	setHasGetterSetterProperties(true);	299	setHasGetterSetterProperties(true);

Lines 1-5 Source/JavaScriptCore/runtime/StructureInlines.h_sec1
- Source/JavaScriptCore/runtime/StructureInlines.h -1 / +30 lines
1	/*	1	/*
2	* Copyright (C) 2013 Apple Inc. All rights reserved.	2	* Copyright (C) 2013, 2014 Apple Inc. All rights reserved.
3	*	3	*
4	* Redistribution and use in source and binary forms, with or without	4	* Redistribution and use in source and binary forms, with or without
5	* modification, are permitted provided that the following conditions	5	* modification, are permitted provided that the following conditions
Lines 104-109 ALWAYS_INLINE PropertyOffset Structure:: Source/JavaScriptCore/runtime/StructureInlines.h_sec2
104	return entry->offset;	104	return entry->offset;
105	}	105	}
106		106
		107	template<typename Functor>
		108	void Structure::forEachPropertyConcurrently(const Functor& functor)
		109	{
		110	Vector<Structure*, 8> structures;
		111	Structure* structure;
		112	PropertyTable* table;
		113
		114	findStructuresAndMapForMaterialization(structures, structure, table);
		115
		116	if (table) {
		117	for (auto entry : *table) {
		118	if (!functor(entry)) {
		119	structure->m_lock.unlock();
		120	return;
		121	}
		122	}
		123	structure->m_lock.unlock();
		124	}
		125
		126	for (unsigned i = structures.size(); i--;) {
		127	structure = structures[i];
		128	if (!structure->m_nameInPrevious)
		129	continue;
		130
		131	if (!functor(PropertyMapEntry(structure->m_nameInPrevious.get(), structure->m_offset, structure->attributesInPrevious())))
		132	return;
		133	}
		134	}
		135
107	inline PropertyOffset Structure::getConcurrently(StringImpl* uid)	136	inline PropertyOffset Structure::getConcurrently(StringImpl* uid)
108	{	137	{
109	unsigned attributesIgnored;	138	unsigned attributesIgnored;

Lines 1-3 Source/JavaScriptCore/ChangeLog_sec1
- Source/JavaScriptCore/ChangeLog +18 lines
		1	2014-09-21 Filip Pizlo <fpizlo@apple.com>
		2
		3	Structure should have a method for concurrently getting all of the property map entries, and this method shouldn't involve copy-paste
		4	https://bugs.webkit.org/show_bug.cgi?id=136983
		5
		6	Reviewed by NOBODY (OOPS!).
		7
		8	* runtime/PropertyMapHashTable.h:
		9	(JSC::PropertyMapEntry::PropertyMapEntry): Moved PropertyMapEntry struct to Structure.h so that Structure can refer to it.
		10	* runtime/Structure.cpp:
		11	(JSC::Structure::getConcurrently): Switch to using the new forEachPropertyConcurrently() method.
		12	(JSC::Structure::getPropertiesConcurrently): The subject of this patch. It will be useful for object allocation sinking (bug 136330).
		13	(JSC::Structure::dump): Switch to using the new forEachPropertyConcurrently() method.
		14	* runtime/Structure.h:
		15	(JSC::PropertyMapEntry::PropertyMapEntry): Moved from PropertyMapHashTable.h.
		16	* runtime/StructureInlines.h:
		17	(JSC::Structure::forEachPropertyConcurrently): Capture this very common concurrent structure iteration pattern into a template method.
		18
1	2014-09-21 Filip Pizlo <fpizlo@apple.com>	19	2014-09-21 Filip Pizlo <fpizlo@apple.com>
2		20
3	Structure::getConcurrently() doesn't need to take a VM& argument.	21	Structure::getConcurrently() doesn't need to take a VM& argument.