- Revision
- 173799
- Author
- [email protected]
- Date
- 2014-09-21 16:52:31 -0700 (Sun, 21 Sep 2014)
Log Message
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 Mark Hahnenberg.
* 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.
Modified Paths
Diff
Modified: trunk/Source/_javascript_Core/ChangeLog (173798 => 173799)
--- trunk/Source/_javascript_Core/ChangeLog 2014-09-21 20:30:10 UTC (rev 173798)
+++ trunk/Source/_javascript_Core/ChangeLog 2014-09-21 23:52:31 UTC (rev 173799)
@@ -1,5 +1,23 @@
2014-09-21 Filip Pizlo <[email protected]>
+ 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 Mark Hahnenberg.
+
+ * 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 <[email protected]>
+
Structure::getConcurrently() doesn't need to take a VM& argument.
Rubber stamped by Dan Bernstein.
Modified: trunk/Source/_javascript_Core/runtime/PropertyMapHashTable.h (173798 => 173799)
--- trunk/Source/_javascript_Core/runtime/PropertyMapHashTable.h 2014-09-21 20:30:10 UTC (rev 173798)
+++ trunk/Source/_javascript_Core/runtime/PropertyMapHashTable.h 2014-09-21 23:52:31 UTC (rev 173799)
@@ -78,19 +78,6 @@
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',
Modified: trunk/Source/_javascript_Core/runtime/Structure.cpp (173798 => 173799)
--- trunk/Source/_javascript_Core/runtime/Structure.cpp 2014-09-21 20:30:10 UTC (rev 173798)
+++ trunk/Source/_javascript_Core/runtime/Structure.cpp 2014-09-21 23:52:31 UTC (rev 173799)
@@ -852,33 +852,32 @@
PropertyOffset Structure::getConcurrently(StringImpl* uid, unsigned& attributes)
{
- Vector<Structure*, 8> structures;
- Structure* structure;
- PropertyTable* table;
+ PropertyOffset result = invalidOffset;
- findStructuresAndMapForMaterialization(structures, structure, table);
+ forEachPropertyConcurrently(
+ [&] (const PropertyMapEntry& candidate) -> bool {
+ if (candidate.key != uid)
+ return true;
+
+ result = candidate.offset;
+ attributes = candidate.attributes;
+ return false;
+ });
- 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();
- }
+ return result;
+}
+
+Vector<PropertyMapEntry> Structure::getPropertiesConcurrently()
+{
+ Vector<PropertyMapEntry> result;
+
+ forEachPropertyConcurrently(
+ [&] (const PropertyMapEntry& entry) -> bool {
+ result.append(entry);
+ return true;
+ });
- for (unsigned i = structures.size(); i--;) {
- structure = structures[i];
- if (structure->m_nameInPrevious.get() != uid)
- continue;
-
- attributes = structure->attributesInPrevious();
- return structure->m_offset;
- }
-
- return invalidOffset;
+ return result;
}
PropertyOffset Structure::add(VM& vm, PropertyName propertyName, unsigned attributes)
@@ -1110,31 +1109,14 @@
{
out.print(RawPointer(this), ":[", classInfo()->className, ", {");
- Vector<Structure*, 8> structures;
- Structure* structure;
- PropertyTable* table;
-
- const_cast<Structure*>(this)->findStructuresAndMapForMaterialization(
- structures, structure, table);
-
CommaPrinter comma;
- if (table) {
- PropertyTable::iterator iter = table->begin();
- PropertyTable::iterator end = table->end();
- for (; iter != end; ++iter)
- out.print(comma, iter->key, ":", static_cast<int>(iter->offset));
-
- structure->m_lock.unlock();
- }
+ const_cast<Structure*>(this)->forEachPropertyConcurrently(
+ [&] (const PropertyMapEntry& entry) -> bool {
+ out.print(comma, entry.key, ":", static_cast<int>(entry.offset));
+ return true;
+ });
- 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()));
if (m_prototype.get().isCell())
Modified: trunk/Source/_javascript_Core/runtime/Structure.h (173798 => 173799)
--- trunk/Source/_javascript_Core/runtime/Structure.h 2014-09-21 20:30:10 UTC (rev 173798)
+++ trunk/Source/_javascript_Core/runtime/Structure.h 2014-09-21 23:52:31 UTC (rev 173799)
@@ -73,6 +73,26 @@
// 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;
@@ -262,9 +282,18 @@
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);
Modified: trunk/Source/_javascript_Core/runtime/StructureInlines.h (173798 => 173799)
--- trunk/Source/_javascript_Core/runtime/StructureInlines.h 2014-09-21 20:30:10 UTC (rev 173798)
+++ trunk/Source/_javascript_Core/runtime/StructureInlines.h 2014-09-21 23:52:31 UTC (rev 173799)
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2013 Apple Inc. All rights reserved.
+ * 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
@@ -104,6 +104,35 @@
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;
