Bug 3917 – opEquals for Ojbect could be more efficient
Status
RESOLVED
Resolution
FIXED
Severity
enhancement
Priority
P2
Component
druntime
Product
D
Version
D2
Platform
Other
OS
All
Creation time
2010-03-09T12:42:00Z
Last change time
2014-02-15T02:42:39Z
Assigned to
sean
Creator
schveiguy
Comments
Comment #0 by schveiguy — 2010-03-09T12:42:34Z
The implementation of opEquals in object_.d has an unnecessary recursive call. The complete implementation is this:
bool opEquals(Object lhs, Object rhs)
{
// If aliased to the same object or both null => equal
if (lhs is rhs) return true;
// If either is null => non-equal
if (lhs is null || rhs is null) return false;
// If same exact type => one call to method opEquals
if (typeid(lhs) == typeid(rhs)) return lhs.opEquals(rhs);
// General case => symmetric calls to method opEquals
return lhs.opEquals(rhs) && rhs.opEquals(lhs);
}
bool opEquals(TypeInfo lhs, TypeInfo rhs)
{
// If aliased to the same object or both null => equal
if (lhs is rhs) return true;
// If either is null => non-equal
if (lhs is null || rhs is null) return false;
// If same exact type => one call to method opEquals
if (typeid(lhs) == typeid(rhs)) return lhs.opEquals(rhs);
// Factor out top level const
// (This still isn't right, should follow same rules as compiler does for type equality.)
TypeInfo_Const c = cast(TypeInfo_Const) lhs;
if (c)
lhs = c.base;
c = cast(TypeInfo_Const) rhs;
if (c)
rhs = c.base;
// General case => symmetric calls to method opEquals
return lhs.opEquals(rhs) && rhs.opEquals(lhs);
}
The third if statement that compares two typeids recurses into opEquals for typeinfos. In the typeinfo comparison, the typeinfos should never be null, and the typeinfos of the typeinfo will always be identical (TypeInfo_Class). In addition, the stuff at the end dealing with const will never come into play because the dynamic typeinfo (the classinfo) never deals with const.
I'd propose to rewrite the opEquals as follows:
bool opEquals(Object lhs, Object rhs)
{
// If aliased to the same object or both null => equal
if (lhs is rhs) return true;
// If either is null => non-equal
if (lhs is null || rhs is null) return false;
// If same exact type => one call to method opEquals
if (typeid(lhs) is typeid(rhs) || typeid(lhs).opEquals(typeid(rhs))) return lhs.opEquals(rhs);
// General case => symmetric calls to method opEquals
return lhs.opEquals(rhs) && rhs.opEquals(lhs);
}
I don't know what else the opEquals that compares typeinfos is for, because it's not a public function, so it could potentially be removed.
Comment #1 by bugzilla — 2010-08-13T01:56:44Z
Fixed DMD 2.048.
Comment #2 by github-bugzilla — 2013-03-09T21:34:17Z