From 09c916e585f4c7e35e2eab206bb9dfc0141547cc Mon Sep 17 00:00:00 2001 From: Paul Tarjan Date: Wed, 12 Jun 2013 23:53:26 -0700 Subject: [PATCH] make o_toBoolean non-virtual I know this is bad form, but we need this for performance. Right now, o_toBoolean() isn't even being called. When D825610 lands it will be and will be a small perf regression. If this situation gets bad, then we should revisit it, but since only one class is overriding it for now, this should be ok. (I don't understand C++, did I do the virtual thing right? I had to declare them to make the linker work) --- hphp/runtime/base/object_data.cpp | 13 ++++++++---- hphp/runtime/base/object_data.h | 32 ++++++++++++++++++++++++++--- hphp/runtime/base/resource_data.cpp | 1 + hphp/runtime/base/resource_data.h | 4 +++- hphp/runtime/ext/ext_simplexml.cpp | 9 ++++---- hphp/runtime/ext/ext_simplexml.h | 9 ++++---- 6 files changed, 52 insertions(+), 16 deletions(-) diff --git a/hphp/runtime/base/object_data.cpp b/hphp/runtime/base/object_data.cpp index 340f5bd45..6bef9b4e7 100644 --- a/hphp/runtime/base/object_data.cpp +++ b/hphp/runtime/base/object_data.cpp @@ -25,6 +25,7 @@ #include "hphp/runtime/ext/ext_closure.h" #include "hphp/runtime/ext/ext_continuation.h" #include "hphp/runtime/ext/ext_collections.h" +#include "hphp/runtime/ext/ext_simplexml.h" #include "hphp/runtime/vm/class.h" namespace HPHP { @@ -117,10 +118,14 @@ bool ObjectData::o_instanceof(CStrRef s) const { return m_cls->classof(cls); } -int64_t ObjectData::o_toInt64() const { - raise_notice("Object of class %s could not be converted to int", - o_getClassName().data()); - return 1; +bool ObjectData::o_toBooleanImpl() const noexcept { + not_reached(); +} +int64_t ObjectData::o_toInt64Impl() const noexcept { + not_reached(); +} +double ObjectData::o_toDoubleImpl() const noexcept { + not_reached(); } /////////////////////////////////////////////////////////////////////////////// diff --git a/hphp/runtime/base/object_data.h b/hphp/runtime/base/object_data.h index 406ddd678..eeddde01e 100644 --- a/hphp/runtime/base/object_data.h +++ b/hphp/runtime/base/object_data.h @@ -60,6 +60,7 @@ class ObjectData : public CountableNF { UseUnset = 0x0020, // __unset() HasCall = 0x0080, // defines __call HasCallStatic = 0x0100, // defines __callStatic + CallToImpl = 0x0200, // call o_to{Boolean,Int64,Double}Impl // The top 3 bits of o_attributes are reserved to indicate the // type of collection CollectionTypeAttrMask = (7 << 13), @@ -142,10 +143,35 @@ class ObjectData : public CountableNF { virtual bool isResource() const { return false; } int o_getId() const { return o_id;} + bool o_toBoolean() const { + if (getAttribute(CallToImpl)) { + return o_toBooleanImpl(); + } + return true; + } + + int64_t o_toInt64() const { + if (getAttribute(CallToImpl)) { + return o_toInt64Impl(); + } + raise_notice( + "Object of class %s could not be converted to int", + o_getClassName().data() + ); + return 1; + } + + double o_toDouble() const { + if (getAttribute(CallToImpl)) { + return o_toDoubleImpl(); + } + return o_toInt64(); + } + // overridable casting - virtual bool o_toBoolean() const { return true;} - virtual int64_t o_toInt64() const; - virtual double o_toDouble() const { return o_toInt64();} + virtual bool o_toBooleanImpl() const noexcept; + virtual int64_t o_toInt64Impl() const noexcept; + virtual double o_toDoubleImpl() const noexcept; void destruct(); diff --git a/hphp/runtime/base/resource_data.cpp b/hphp/runtime/base/resource_data.cpp index 0466a830b..9f1ff4a81 100644 --- a/hphp/runtime/base/resource_data.cpp +++ b/hphp/runtime/base/resource_data.cpp @@ -33,6 +33,7 @@ int ResourceData::GetMaxResourceId() { ResourceData::ResourceData() : Instance(SystemLib::s_resourceClass, true) { assert(!m_cls->callsCustomInstanceInit()); + ObjectData::setAttributes(ObjectData::CallToImpl); int &pmax = *os_max_resource_id; if (pmax < 3) pmax = 3; // reserving 1, 2, 3 for STDIN, STDOUT, STDERR o_id = ++pmax; diff --git a/hphp/runtime/base/resource_data.h b/hphp/runtime/base/resource_data.h index 4e455f36c..5e23b9538 100644 --- a/hphp/runtime/base/resource_data.h +++ b/hphp/runtime/base/resource_data.h @@ -37,7 +37,9 @@ public: // implementing ObjectData virtual bool isResource() const { return true;} virtual String t___tostring(); - virtual int64_t o_toInt64() const { return o_getId();} + virtual bool o_toBooleanImpl() const noexcept { return 1; } + virtual int64_t o_toInt64Impl() const noexcept { return o_getId(); } + virtual double o_toDoubleImpl() const noexcept { return o_getId(); } void o_setId(int id); // only for BuiltinFiles virtual void serializeImpl(VariableSerializer *serializer) const; virtual CStrRef o_getResourceName() const; diff --git a/hphp/runtime/ext/ext_simplexml.cpp b/hphp/runtime/ext/ext_simplexml.cpp index 4f047c387..f3d589d17 100644 --- a/hphp/runtime/ext/ext_simplexml.cpp +++ b/hphp/runtime/ext/ext_simplexml.cpp @@ -336,7 +336,8 @@ c_SimpleXMLElement::c_SimpleXMLElement(Class* cb) : ExtObjectDataFlags(cb), + ObjectData::UseUnset| + ObjectData::CallToImpl>(cb), m_node(NULL), m_is_text(false), m_free_text(false), m_is_attribute(false), m_is_children(false), m_is_property(false), m_xpath(NULL) { @@ -887,11 +888,11 @@ Variant c_SimpleXMLElement::t___set(Variant name, Variant value) { return uninit_null(); } -bool c_SimpleXMLElement::o_toBoolean() const { +bool c_SimpleXMLElement::o_toBooleanImpl() const noexcept { return m_node != NULL || getProperties().size(); } -int64_t c_SimpleXMLElement::o_toInt64() const { +int64_t c_SimpleXMLElement::o_toInt64Impl() const noexcept { Variant prop; ArrayIter iter(m_children); if (iter) { @@ -900,7 +901,7 @@ int64_t c_SimpleXMLElement::o_toInt64() const { return prop.toString().toInt64(); } -double c_SimpleXMLElement::o_toDouble() const { +double c_SimpleXMLElement::o_toDoubleImpl() const noexcept { Variant prop; ArrayIter iter(m_children); if (iter) { diff --git a/hphp/runtime/ext/ext_simplexml.h b/hphp/runtime/ext/ext_simplexml.h index cf517e2b1..2cd84302b 100644 --- a/hphp/runtime/ext/ext_simplexml.h +++ b/hphp/runtime/ext/ext_simplexml.h @@ -44,7 +44,8 @@ class c_SimpleXMLElement : public ExtObjectDataFlags, + ObjectData::UseUnset| + ObjectData::CallToImpl>, public Sweepable { public: DECLARE_CLASS(SimpleXMLElement, SimpleXMLElement, ObjectData) @@ -86,9 +87,9 @@ class c_SimpleXMLElement : bool m_is_attribute; bool m_is_children; bool m_is_property; - virtual bool o_toBoolean() const; - virtual int64_t o_toInt64() const; - virtual double o_toDouble() const; + virtual bool o_toBooleanImpl() const noexcept; + virtual int64_t o_toInt64Impl() const noexcept; + virtual double o_toDoubleImpl() const noexcept; virtual Array o_toArray() const; private: xmlXPathContextPtr m_xpath;