Reduce use of AttachLiteral StringData construction

This is the first results from profiling callsites of
StringData::initLiteral.  This diff converts a handful more
string literals to StaticString, removes overloaded Variant
comparison operators (operator==, etc), and avoids
constructing new strings in a few cases in tvCastToString
and tvCastToStringInPlace.
Esse commit está contido em:
Edwin Smith
2013-06-04 17:10:13 -07:00
commit de Sara Golemon
commit fe32089a67
20 arquivos alterados com 133 adições e 129 exclusões
+2
Ver Arquivo
@@ -53,6 +53,8 @@ int64_t VMExecutionContext::s_threadIdxCounter = 0;
Mutex VMExecutionContext::s_threadIdxLock;
hphp_hash_map<pid_t, int64_t> VMExecutionContext::s_threadIdxMap;
const StaticString BaseExecutionContext::s_amp("&");
BaseExecutionContext::BaseExecutionContext() :
m_fp(nullptr), m_pc(nullptr),
m_transport(nullptr),
+2 -1
Ver Arquivo
@@ -324,7 +324,7 @@ public:
void setTimeZone(CStrRef timezone) { m_timezone = timezone;}
String getDefaultTimeZone() const { return m_timezoneDefault;}
String getArgSeparatorOutput() const {
if (m_argSeparatorOutput.isNull()) return "&";
if (m_argSeparatorOutput.isNull()) return s_amp;
return m_argSeparatorOutput;
}
void setArgSeparatorOutput(CStrRef s) { m_argSeparatorOutput = s;}
@@ -353,6 +353,7 @@ private:
};
private:
static const StaticString s_amp;
// system settings
Transport *m_transport;
int64_t m_maxMemory;
+36 -16
Ver Arquivo
@@ -14,10 +14,11 @@
+----------------------------------------------------------------------+
*/
#include "hphp/runtime/base/ini_setting.h"
#define __STDC_LIMIT_MACROS
#include <stdint.h>
#include "hphp/runtime/base/ini_setting.h"
#include "hphp/runtime/base/complex_types.h"
#include "hphp/runtime/base/type_conversions.h"
#include "hphp/runtime/base/builtin_functions.h"
@@ -208,64 +209,83 @@ void IniSetting::Unbind(const char *name) {
s_callbacks->erase(name);
}
static const StaticString
s_error_reporting("error_reporting"),
s_memory_limit("memory_limit"),
s_max_execution_time("max_execution_time"),
s_maximum_execution_time("maximum_execution_time"),
s_hphp_build_id("hphp.build_id"),
s_hphp_compiler_version("hphp.compiler_version"),
s_hphp_compiler_id("hphp.compiler_id"),
s_arg_separator_output("arg_separator.output"),
s_upload_max_filesize("upload_max_filesize"),
s_post_max_size("post_max_size"),
s_log_errors("log_errors"),
s_error_log("error_log"),
s_notice_frequency("notice_frequency"),
s_warning_frequency("warning_frequency"),
s_include_path("include_path"),
s_1("1"),
s_0("0");
bool IniSetting::Get(CStrRef name, String &value) {
if (name == "error_reporting") {
if (name == s_error_reporting) {
value = String((int64_t)g_context->getErrorReportingLevel());
return true;
}
if (name == "memory_limit") {
if (name == s_memory_limit) {
int64_t v = g_context->getRequestMemoryMaxBytes();
if (v == INT64_MAX) v = -1;
value = String(v);
return true;
}
if (name == "max_execution_time" || name == "maximum_execution_time") {
if (name == s_max_execution_time || name == s_maximum_execution_time) {
value = String((int64_t)g_context->getRequestTimeLimit());
return true;
}
if (name == "hphp.build_id") {
if (name == s_hphp_build_id) {
value = String(RuntimeOption::BuildId);
return true;
}
if (name == "hphp.compiler_version") {
if (name == s_hphp_compiler_version) {
value = String(getHphpCompilerVersion());
return true;
}
if (name == "hphp.compiler_id") {
if (name == s_hphp_compiler_id) {
value = String(getHphpCompilerId());
return true;
}
if (name == "arg_separator.output") {
if (name == s_arg_separator_output) {
value = g_context->getArgSeparatorOutput();
return true;
}
if (name == "upload_max_filesize") {
if (name == s_upload_max_filesize) {
int uploadMaxFilesize = VirtualHost::GetUploadMaxFileSize() / (1 << 20);
value = String(uploadMaxFilesize) + "M";
return true;
}
if (name == "post_max_size") {
if (name == s_post_max_size) {
int postMaxSize = VirtualHost::GetMaxPostSize();
value = String(postMaxSize);
return true;
}
if (name == "log_errors") {
value = g_context->getLogErrors() ? "1" : "0";
if (name == s_log_errors) {
value = g_context->getLogErrors() ? s_1 : s_0;
return true;
}
if (name == "error_log") {
if (name == s_error_log) {
value = g_context->getErrorLog();
return true;
}
if (name == "notice_frequency") {
if (name == s_notice_frequency) {
value = String((int64_t)RuntimeOption::NoticeFrequency);
return true;
}
if (name == "warning_frequency") {
if (name == s_warning_frequency) {
value = String((int64_t)RuntimeOption::WarningFrequency);
return true;
}
if (name == "include_path") {
if (name == s_include_path) {
value = g_context->getIncludePath();
return true;
}
+2 -2
Ver Arquivo
@@ -317,10 +317,10 @@ void ObjectData::o_setArray(CArrRef properties) {
Class* ctx = nullptr;
// If the key begins with a NUL, it's a private or protected property. Read
// the class name from between the two NUL bytes.
if (!k.empty() && k.charAt(0) == '\0') {
if (!k.empty() && k[0] == '\0') {
int subLen = k.find('\0', 1) + 1;
String cls = k.substr(1, subLen - 2);
if (cls == "*") {
if (cls.size() == 1 && cls[0] == '*') {
// Protected.
ctx = m_cls;
} else {
+25 -11
Ver Arquivo
@@ -142,6 +142,10 @@ void tvCastToDoubleInPlace(TypedValue* tv) {
tv->m_type = KindOfDouble;
}
static const StaticString
s_1("1"),
s_Array("Array");
void tvCastToStringInPlace(TypedValue* tv) {
if (tv->m_type == KindOfRef) {
tvUnbox(tv);
@@ -149,23 +153,33 @@ void tvCastToStringInPlace(TypedValue* tv) {
StringData * s;
switch (tv->m_type) {
case KindOfUninit:
case KindOfNull: s = buildStringData(""); break;
case KindOfBoolean: s = buildStringData(tv->m_data.num ? "1" : ""); break;
case KindOfNull: s = empty_string.get(); goto static_string;
case KindOfBoolean:
s = tv->m_data.num ? s_1.get() : empty_string.get();
goto static_string;
case KindOfInt64: s = buildStringData(tv->m_data.num); break;
case KindOfDouble: s = buildStringData(tv->m_data.dbl); break;
case KindOfStaticString:
case KindOfString: return;
case KindOfArray: s = buildStringData("Array"); tvDecRefArr(tv); break;
case KindOfObject: {
case KindOfArray:
s = s_Array.get();
tvDecRefArr(tv);
goto static_string;
case KindOfObject:
// For objects, we fall back on the Variant machinery
tvAsVariant(tv) = tv->m_data.pobj->t___tostring();
return;
default:
not_reached();
}
default: not_reached();
}
s->incRefCount();
tv->m_data.pstr = s;
tv->m_type = KindOfString;
tv->m_data.pstr->incRefCount();
return;
static_string:
tv->m_data.pstr = s;
tv->m_type = KindOfStaticString;
}
StringData* tvCastToString(TypedValue* tv) {
@@ -176,13 +190,13 @@ StringData* tvCastToString(TypedValue* tv) {
StringData* s;
switch (tv->m_type) {
case KindOfUninit:
case KindOfNull: s = buildStringData(""); break;
case KindOfBoolean: s = buildStringData(tv->m_data.num ? "1" : ""); break;
case KindOfNull: return empty_string.get();
case KindOfBoolean: return tv->m_data.num ? s_1.get() : empty_string.get();
case KindOfInt64: s = buildStringData(tv->m_data.num); break;
case KindOfDouble: s = buildStringData(tv->m_data.dbl); break;
case KindOfStaticString:
case KindOfStaticString: return tv->m_data.pstr;
case KindOfString: s = tv->m_data.pstr; break;
case KindOfArray: return StringData::GetStaticString("Array");
case KindOfArray: return s_Array.get();
case KindOfObject: return tv->m_data.pobj->t___tostring().detach();
default: not_reached();
}
-11
Ver Arquivo
@@ -87,17 +87,6 @@ public:
if (it != integer_string_data_map.end()) return it->second;
return nullptr;
}
static const StringData *GetIntegerStringData(int n) {
return GetIntegerStringData((int64_t)n);
}
static const char *GetIntegerString(int64_t n) {
const StringData *sd = GetIntegerStringData(n);
if (sd) return sd->data();
return nullptr;
}
static const char *GetIntegerString(int n) {
return GetIntegerString((int64_t)n);
}
public:
String() {}
+11 -28
Ver Arquivo
@@ -131,12 +131,12 @@ Variant::Variant(CObjRef v) {
HOT_FUNC
Variant::Variant(StringData *v) {
m_type = KindOfString;
if (v) {
m_data.pstr = v;
if (v->isStatic()) {
m_type = KindOfStaticString;
} else {
m_type = KindOfString;
v->incRefCount();
}
} else {
@@ -144,6 +144,16 @@ Variant::Variant(StringData *v) {
}
}
Variant::Variant(const StringData *v) {
if (v) {
assert(v->isStatic());
m_data.pstr = const_cast<StringData*>(v);
m_type = KindOfStaticString;
} else {
m_type = KindOfNull;
}
}
Variant::Variant(ArrayData *v) {
m_type = KindOfArray;
if (v) {
@@ -1790,33 +1800,6 @@ bool Variant::more(CArrRef v2) const { UNWRAP_ARR(more,less);}
bool Variant::more(CObjRef v2) const { UNWRAP(less);}
bool Variant::more(CVarRef v2) const { UNWRAP_VAR(more,less);}
///////////////////////////////////////////////////////////////////////////////
// comparison operators
bool Variant::operator==(CVarRef v) const {
return HPHP::equal(*this, v);
}
bool Variant::operator!=(CVarRef v) const {
return !HPHP::equal(*this, v);
}
bool Variant::operator>=(CVarRef v) const {
return more_or_equal(*this, v);
}
bool Variant::operator<=(CVarRef v) const {
return less_or_equal(*this, v);
}
bool Variant::operator>(CVarRef v) const {
return HPHP::more(*this, v);
}
bool Variant::operator<(CVarRef v) const {
return HPHP::less(*this, v);
}
///////////////////////////////////////////////////////////////////////////////
// offset functions
+3 -13
Ver Arquivo
@@ -138,6 +138,9 @@ class Variant : private TypedValue {
/* implicit */ Variant(ObjectData *v);
/* implicit */ Variant(RefData *r);
// for static strings only
explicit Variant(const StringData *v);
// Move ctor for strings
/* implicit */ Variant(String&& v) {
StringData *s = v.get();
@@ -179,7 +182,6 @@ class Variant : private TypedValue {
// These are prohibited, but declared just to prevent accidentally
// calling the bool constructor just because we had a pointer to
// const.
/* implicit */ Variant(const StringData *v) = delete;
/* implicit */ Variant(const ArrayData *v) = delete;
/* implicit */ Variant(const ObjectData *v) = delete;
/* implicit */ Variant(const RefData *v) = delete;
@@ -598,18 +600,6 @@ class Variant : private TypedValue {
Variant &operator -- ();
Variant operator -- (int);
/**
* These are convenient functions for writing extensions, since code
* generation always uses explicit functions like same(), less() etc. that
* are type specialized and unambiguous.
*/
bool operator == (CVarRef v) const;
bool operator != (CVarRef v) const;
bool operator >= (CVarRef v) const;
bool operator <= (CVarRef v) const;
bool operator > (CVarRef v) const;
bool operator < (CVarRef v) const;
/**
* Iterator functions. See array_iterator.h for end() and next().
*/
+7 -12
Ver Arquivo
@@ -52,7 +52,8 @@ bool f_function_exists(CStrRef function_name, bool autoload /* = true */) {
static const StaticString
s__invoke("__invoke"),
s_Closure__invoke("Closure::__invoke");
s_Closure__invoke("Closure::__invoke"),
s_colon2("::");
bool f_is_callable(CVarRef v, bool syntax /* = false */,
VRefParam name /* = null */) {
@@ -106,7 +107,7 @@ bool f_is_callable(CVarRef v, bool syntax /* = false */,
return false;
}
name = concat3(name, "::", Variant::GetAsString(tv_meth));
name = concat3(name, s_colon2, Variant::GetAsString(tv_meth));
return ret;
}
@@ -255,17 +256,11 @@ Variant f_forward_static_call(int _argc, CVarRef function,
Variant f_get_called_class() {
EagerCallerFrame cf;
ActRec* ar = cf();
if (ar == NULL) {
return Variant(false);
}
if (ar->hasThis()) {
ObjectData* obj = ar->getThis();
return obj->o_getClassName();
} else if (ar->hasClass()) {
return ar->getClass()->preClass()->name()->data();
} else {
return Variant(false);
if (ar) {
if (ar->hasThis()) return Variant(ar->getThis()->o_getClassName());
if (ar->hasClass()) return Variant(ar->getClass()->preClass()->name());
}
return Variant(false);
}
String f_create_function(CStrRef args, CStrRef code) {
+6 -4
Ver Arquivo
@@ -1574,9 +1574,11 @@ String f_image_type_to_extension(int imagetype,
}
}
static const StaticString s_bits("bits");
static const StaticString s_channels("channels");
static const StaticString s_mime("mime");
static const StaticString
s_bits("bits"),
s_channels("channels"),
s_mime("mime"),
s_linespacing("linespacing");
Variant f_getimagesize(CStrRef filename, VRefParam imageinfo /* = null */) {
int itype = 0;
@@ -2872,7 +2874,7 @@ static Variant php_imagettftext_common(int mode, int extended,
Variant key = iter.first();
if (!key.isString()) continue;
Variant item = iter.second();
if (key == "linespacing") {
if (equal(key, s_linespacing)) {
strex.flags |= gdFTEX_LINESPACE;
strex.linespacing = toDouble(item);
}
+1 -1
Ver Arquivo
@@ -657,7 +657,7 @@ Array f_ini_get_all(CStrRef extension /* = null_string */) {
}
String f_ini_get(CStrRef varname) {
String value("");
String value = empty_string;
IniSetting::Get(varname, value);
return value;
}
+3 -1
Ver Arquivo
@@ -171,6 +171,8 @@ static void url_encode_array(StringBuffer &ret, CVarRef varr,
}
}
static const StaticString s_arg_separator_output("arg_separator.output");
Variant f_http_build_query(CVarRef formdata,
CStrRef numeric_prefix /* = null_string */,
CStrRef arg_separator /* = null_string */) {
@@ -181,7 +183,7 @@ Variant f_http_build_query(CVarRef formdata,
String arg_sep;
if (arg_separator.isNull()) {
arg_sep = f_ini_get("arg_separator.output");
arg_sep = f_ini_get(s_arg_separator_output);
} else {
arg_sep = arg_separator;
}
+7 -5
Ver Arquivo
@@ -84,6 +84,8 @@ void throw_tprotocolexception(CStrRef what, long errorcode) {
throw ex;
}
static const StaticString s_TSPEC("_TSPEC");
Variant binary_deserialize(int8_t thrift_typeID, PHPInputTransport& transport,
CArrRef fieldspec) {
Variant ret;
@@ -105,7 +107,7 @@ Variant binary_deserialize(int8_t thrift_typeID, PHPInputTransport& transport,
skip_element(T_STRUCT, transport);
return uninit_null();
}
Variant spec = f_hphp_get_static_property(structType, "_TSPEC");
Variant spec = f_hphp_get_static_property(structType, s_TSPEC);
if (!spec.is(KindOfArray)) {
char errbuf[128];
snprintf(errbuf, 128, "spec for %s is wrong type: %d\n",
@@ -357,7 +359,7 @@ void binary_serialize(int8_t thrift_typeID, PHPOutputTransport& transport,
binary_serialize_spec(value, transport,
f_hphp_get_static_property(toObject(value)->
o_getClassName(),
"_TSPEC").toArray());
s_TSPEC).toArray());
} return;
case T_BOOL:
transport.writeI8(value.toBoolean() ? 1 : 0);
@@ -488,7 +490,7 @@ void f_thrift_protocol_write_binary(CObjRef transportobj, CStrRef method_name,
}
Variant spec = f_hphp_get_static_property(request_struct->o_getClassName(),
"_TSPEC");
s_TSPEC);
binary_serialize_spec(request_struct, transport, spec.toArray());
transport.flush();
@@ -524,13 +526,13 @@ Variant f_thrift_protocol_read_binary(CObjRef transportobj,
if (messageType == T_EXCEPTION) {
Object ex = createObject("TApplicationException");
Variant spec = f_hphp_get_static_property("TApplicationException", "_TSPEC");
Variant spec = f_hphp_get_static_property("TApplicationException", s_TSPEC);
binary_deserialize_spec(ex, transport, spec.toArray());
throw ex;
}
Object ret_val = createObject(obj_typename);
Variant spec = f_hphp_get_static_property(obj_typename, "_TSPEC");
Variant spec = f_hphp_get_static_property(obj_typename, s_TSPEC);
binary_deserialize_spec(ret_val, transport, spec.toArray());
return ret_val;
}
+5 -4
Ver Arquivo
@@ -1633,17 +1633,18 @@ Array VMExecutionContext::getLocalDefinedVariables(int frame) {
if (fp->hasVarEnv()) {
return fp->m_varEnv->getDefinedVariables();
}
Array ret = Array::Create();
const Func *func = fp->m_func;
for (Id id = 0; id < func->numNamedLocals(); ++id) {
auto numLocals = func->numNamedLocals();
ArrayInit ret(numLocals);
for (Id id = 0; id < numLocals; ++id) {
TypedValue* ptv = frame_local(fp, id);
if (ptv->m_type == KindOfUninit) {
continue;
}
Variant name(func->localVarName(id)->data());
Variant name(func->localVarName(id));
ret.add(name, tvAsVariant(ptv));
}
return ret;
return ret.toArray();
}
void VMExecutionContext::shuffleMagicArgs(ActRec* ar) {
+3 -1
Ver Arquivo
@@ -29,9 +29,11 @@ StringData* prepareAnyKey(TypedValue* tv) {
}
}
const StaticString s_ArrayAccess("ArrayAccess");
void objArrayAccess(Instance* base) {
assert(!base->isCollection());
if (!instanceOf(base, "ArrayAccess")) {
if (!instanceOf(base, s_ArrayAccess)) {
raise_error("Object does not implement ArrayAccess");
}
}
+11 -11
Ver Arquivo
@@ -243,7 +243,7 @@ bool TestCppBase::TestArray() {
arr = Array::Create("test");
VERIFY(!arr.empty()); VERIFY(arr.size() == 1); VERIFY(arr.length() == 1);
VERIFY(!arr.isNull());
VERIFY(arr[0] == "test");
VERIFY(equal(arr[0], String("test")));
VS(arr, Array(ArrayInit(1).set("test").create()));
Array arrCopy = arr;
@@ -263,7 +263,7 @@ bool TestCppBase::TestArray() {
arr = Array::Create(s_name, "test");
VERIFY(!arr.empty()); VERIFY(arr.size() == 1); VERIFY(arr.length() == 1);
VERIFY(!arr.isNull());
VERIFY(arr[s_name] == "test");
VERIFY(equal(arr[s_name], String("test")));
VS(arr, Array(ArrayInit(1).set(s_name, "test").create()));
arrCopy = arr;
@@ -281,11 +281,11 @@ bool TestCppBase::TestArray() {
int i = 0;
for (ArrayIter iter = arr.begin(); iter; ++iter, ++i) {
if (i == 0) {
VERIFY(iter.first() == "n1");
VERIFY(iter.second() == "v1");
VERIFY(equal(iter.first(), String("n1")));
VERIFY(equal(iter.second(), String("v1")));
} else {
VERIFY(iter.first() == "n2");
VERIFY(iter.second() == "v2");
VERIFY(equal(iter.first(), String("n2")));
VERIFY(equal(iter.second(), String("v2")));
}
}
VERIFY(i == 2);
@@ -646,7 +646,7 @@ bool TestCppBase::TestVariant() {
v += 20;
VERIFY(v.isNumeric());
VERIFY(v.is(KindOfInt64));
VERIFY(v == Variant(35));
VERIFY(equal(v, Variant(35)));
}
// conversions
@@ -664,8 +664,8 @@ bool TestCppBase::TestVariant() {
Variant v;
v.lvalAt(0) = String("v0");
v.lvalAt(1) = String("v1");
VERIFY(v[0] == "v0");
VERIFY(v[1] == "v1");
VERIFY(equal(v[0], String("v0")));
VERIFY(equal(v[1], String("v1")));
}
{
Variant v;
@@ -753,13 +753,13 @@ bool TestCppBase::TestVariant() {
Variant v1("original");
Variant v2 = v1;
v2 = String("changed");
VERIFY(v1 == "original");
VERIFY(equal(v1, String("original")));
}
{
Variant v1("original");
Variant v2 = strongBind(v1);
v2 = String("changed");
VERIFY(v1 == "changed");
VERIFY(equal(v1, String("changed")));
}
{
Variant v1 = 10;
+3 -3
Ver Arquivo
@@ -213,9 +213,9 @@ bool TestExtCurl::test_curl_error() {
f_curl_setopt(c, k_CURLOPT_RETURNTRANSFER, true);
f_curl_exec(c);
Variant err = f_curl_error(c);
VERIFY(err == "Couldn't resolve host 'www.thereisnosuchanurl'" ||
err == "Could not resolve host: www.thereisnosuchanurl"
" (Domain name not found)");
VERIFY(equal(err, String("Couldn't resolve host 'www.thereisnosuchanurl'")) ||
equal(err, String("Could not resolve host: www.thereisnosuchanurl"
" (Domain name not found)")));
return Count(true);
}
+1 -1
Ver Arquivo
@@ -66,7 +66,7 @@ bool TestExtNetwork::RunTests(const std::string &which) {
///////////////////////////////////////////////////////////////////////////////
bool TestExtNetwork::test_gethostname() {
VERIFY(f_gethostname() != Variant(false));
VERIFY(!equal(f_gethostname(), Variant(false)));
return Count(true);
}
+2 -1
Ver Arquivo
@@ -107,7 +107,8 @@ static const StaticString
bool TestExtOpenssl::test_openssl_csr_get_subject() {
Variant csr = f_openssl_csr_new(uninit_null(), uninit_null());
VERIFY(!csr.isNull());
VERIFY(f_openssl_csr_get_subject(csr)[s_O] == "Internet Widgits Pty Ltd");
VERIFY(equal(f_openssl_csr_get_subject(csr)[s_O],
String("Internet Widgits Pty Ltd")));
return Count(true);
}
+3 -3
Ver Arquivo
@@ -279,7 +279,7 @@ bool TestExtZlib::test_nzcompress() {
bool TestExtZlib::test_nzuncompress() {
String s("garbage stuff", AttachLiteral);
Variant v = f_nzuncompress(s);
if (v != Variant(false)) {
if (!equal(v, Variant(false))) {
return Count(false);
}
@@ -309,13 +309,13 @@ bool TestExtZlib::test_lz4uncompress() {
// first test uncompressing invalid string
String s("invalid compressed string", AttachLiteral);
Variant v = f_lz4uncompress(s);
if (v != Variant(false)) {
if (!equal(v, Variant(false))) {
return Count(false);
}
// try uncompressing empty string
String empty("", AttachLiteral);
v = f_lz4uncompress(empty);
if (v != Variant(false)) {
if (!equal(v, Variant(false))) {
return Count(false);
}