diff --git a/hphp/compiler/analysis/function_scope.cpp b/hphp/compiler/analysis/function_scope.cpp index c4ee90f33..80f2f026f 100644 --- a/hphp/compiler/analysis/function_scope.cpp +++ b/hphp/compiler/analysis/function_scope.cpp @@ -775,8 +775,7 @@ void FunctionScope::setParamName(int index, const std::string &name) { void FunctionScope::setParamDefault(int index, const char* value, int64_t len, const std::string &text) { assert(index >= 0 && index < (int)m_paramNames.size()); - StringData* sd = new StringData(value, len, AttachLiteral); - sd->setStatic(); + auto sd = StringData::GetStaticString(value, len); m_paramDefaults[index] = String(sd); m_paramDefaultTexts[index] = text; } diff --git a/hphp/runtime/base/string_data.cpp b/hphp/runtime/base/string_data.cpp index 59ebe8eb5..27c52c3e6 100644 --- a/hphp/runtime/base/string_data.cpp +++ b/hphp/runtime/base/string_data.cpp @@ -72,6 +72,17 @@ const StringData* StringData::convert_integer_helper(int64_t n) { return StringData::GetStaticString(p); } +// If a string is static it better be the one in the table. +#ifndef NDEBUG +static bool checkStaticStr(const StringData* s) { + assert(s->isStatic()); + StringDataMap::const_iterator it = s_stringDataMap->find(s); + assert(it != s_stringDataMap->end()); + assert(it->first == s); + return true; +} +#endif + size_t StringData::GetStaticStringCount() { if (!s_stringDataMap) return 0; return s_stringDataMap->size(); @@ -85,6 +96,10 @@ StringData *StringData::GetStaticString(const StringData *str) { new StringDataMap(RuntimeOption::EvalInitialStaticStringTableSize, config); } + if (str->isStatic()) { + assert(checkStaticStr(str)); + return const_cast(str); + } StringDataMap::const_iterator it = s_stringDataMap->find(str); if (it != s_stringDataMap->end()) { return const_cast(it->first); @@ -105,6 +120,10 @@ StringData *StringData::GetStaticString(const StringData *str) { StringData *StringData::LookupStaticString(const StringData *str) { if (UNLIKELY(!s_stringDataMap)) return nullptr; + if (str->isStatic()) { + assert(checkStaticStr(str)); + return const_cast(str); + } StringDataMap::const_iterator it = s_stringDataMap->find(str); if (it != s_stringDataMap->end()) { return const_cast(it->first); @@ -117,29 +136,17 @@ StringData* StringData::GetStaticString(const String& str) { return GetStaticString(str.get()); } -StringData* StringData::FindStaticString(const StringData* str) { - if (UNLIKELY(!s_stringDataMap)) { - StringDataMap::Config config; - config.growthFactor = 1; - s_stringDataMap = - new StringDataMap(RuntimeOption::EvalInitialStaticStringTableSize, - config); - } - StringDataMap::const_iterator it = s_stringDataMap->find(str); - if (it != s_stringDataMap->end()) { - return const_cast(it->first); - } - return nullptr; +StringData *StringData::GetStaticString(const char *str, size_t len) { + StackStringData sd(str, len, AttachLiteral); + return GetStaticString(&sd); } StringData *StringData::GetStaticString(const std::string &str) { - StackStringData sd(str.c_str(), str.size(), AttachLiteral); - return GetStaticString(&sd); + return GetStaticString(str.c_str(), str.size()); } StringData *StringData::GetStaticString(const char *str) { - StackStringData sd(str, strlen(str), AttachLiteral); - return GetStaticString(&sd); + return GetStaticString(str, strlen(str)); } uint32_t StringData::GetCnsHandle(const StringData* cnsName) { diff --git a/hphp/runtime/base/string_data.h b/hphp/runtime/base/string_data.h index d605e9194..3d89f9b26 100644 --- a/hphp/runtime/base/string_data.h +++ b/hphp/runtime/base/string_data.h @@ -122,9 +122,6 @@ class StringData { IMPLEMENT_COUNTABLE_METHODS_NO_STATIC void setRefCount(int32_t n) { _count = n;} - /* Only call preCompute() and setStatic() in a thread-neutral context! */ - void preCompute() const; - void setStatic() const; bool isStatic() const { return _count == RefCountStaticValue; } static StringData *Escalate(StringData *in); @@ -330,10 +327,10 @@ public: void dump() const; std::string toCPPString() const; - static StringData *FindStaticString(const StringData* str); static StringData *GetStaticString(const StringData* str); static StringData *GetStaticString(const std::string& str); static StringData *GetStaticString(const String& str); + static StringData *GetStaticString(const char* str, size_t len); static StringData *GetStaticString(const char* str); static StringData *GetStaticString(char c); @@ -401,6 +398,9 @@ public: assert(!isSmall()); return m_big.cap & ~IsMask; } + /* Only call preCompute() and setStatic() in a thread-neutral context! */ + void preCompute() const; + void setStatic() const; }; /** diff --git a/hphp/runtime/base/type_string.cpp b/hphp/runtime/base/type_string.cpp index 44390a089..d6f67d7c4 100644 --- a/hphp/runtime/base/type_string.cpp +++ b/hphp/runtime/base/type_string.cpp @@ -747,12 +747,5 @@ String& String::operator=(Variant&& src) { return *this = src.toString(); } -void String::checkStaticHelper() { - if (StringData* t = StringData::FindStaticString(m_px)) { - decRefStr(m_px); - m_px = t; - } -} - ////////////////////////////////////////////////////////////////////////////// } diff --git a/hphp/runtime/base/type_string.h b/hphp/runtime/base/type_string.h index 44facc625..ab53fc97f 100644 --- a/hphp/runtime/base/type_string.h +++ b/hphp/runtime/base/type_string.h @@ -194,9 +194,6 @@ public: m_px->setRefCount(1); } - void checkStaticHelper(); - void checkStatic() {} - void clear() { reset();} /** * Informational diff --git a/hphp/runtime/ext/ext_apc.cpp b/hphp/runtime/ext/ext_apc.cpp index 0d7b36600..d8735c88e 100644 --- a/hphp/runtime/ext/ext_apc.cpp +++ b/hphp/runtime/ext/ext_apc.cpp @@ -575,7 +575,6 @@ void apc_load_impl(struct cache_info *info, item.len = (int)(int64_t)*(p+1); // Strings would be copied into APC anyway. String value(*(p+2), (int)(int64_t)*(p+3), AttachLiteral); - value.checkStatic(); s.constructPrime(value, item, false); } s.prime(vars); @@ -858,7 +857,7 @@ void apc_load_impl_compressed p += string_lens[i + i + 2] + 1; // skip \0 // Strings would be copied into APC anyway. String value(p, string_lens[i + i + 3], AttachLiteral); - value.checkStatic(); + // todo: t2539893: check if value is already a static string s.constructPrime(value, item, false); p += string_lens[i + i + 3] + 1; // skip \0 }