Up to Main Index Up to Journal for November, 2019
JOURNAL FOR THURSDAY 21ST NOVEMBER, 2019
______________________________________________________________________________
SUBJECT: In search of a better attribute finder
DATE: Thu 21 Nov 23:52:14 GMT 2019
WolfMUD v0.0.14 was released, and due to a stupid bug, v0.0.15 swiftly
followed. By now the screaming has died down and users are once again happy.
Since then I’ve been casting my eye over the code and decided to address a few
things that have been niggling me for a while. One of those niggles has been
the number of allocations made when the server is just ticking over and
handling mobiles and other events. With the tweaks made so far allocations are
reduced by about 50%.
Part of this has been possible by addressing another major niggle. That being
the attribute finder functions, for example the attr.FindName function. I’ve
had many private discussions with people about the finders. Everybody agrees
the current finders work, but the code is butt-ugly and inefficient. However
nobody could come up with a better way of implementing them :(
I had to find another solution. I tried using reflection and ended up with an
even worse mess. So that idea was thrown out the window.
After a few iterations I ended up adding two new methods to Thing: FindAttr
and FindAttrs. I also added one new method to the has.Attribute interface: Is.
Taking attr.FindName as an example, which finds the Name attribute for a
Thing, the current implementation is:
func FindName(t has.Thing) has.Name {
for _, a := range t.Attrs() {
if a, ok := a.(has.Name); ok {
return a
}
}
return (*Name)(nil)
}
func (t *Thing) Attrs() []has.Attribute {
t.rwmutex.RLock()
a := make([]has.Attribute, len(t.attrs))
for i := range t.attrs {
a[i] = t.attrs[i]
}
t.rwmutex.RUnlock()
return a
}
With the new implementation this becomes:
func FindName(t has.Thing) has.Name {
return t.FindAttr((*Name)(nil)).(has.Name)
}
func (*Name) Is(a has.Attribute) bool {
_, ok := a.(has.Name)
return ok
}
func (t *Thing) FindAttr(cmp has.Attribute) has.Attribute {
t.rwmutex.RLock()
for _, a := range t.attrs {
if cmp.Is(a) {
t.rwmutex.RUnlock()
return a
}
}
t.rwmutex.RUnlock()
return cmp
}
The new implementation does not make a copy of the t.attr slice. It also puts
all of the inner workings into Thing. This makes writing a finder function and
Is method simpler when implementing new attributes.
But… performance is a tad slower, about 5% on average :(
I ran the quiet zone for an hour logging the times to process a command with
the new and old methods. All the times where then averaged for each command:
Command Old Method New Method %Change
--------- ---------- ---------- --------
GET 125.197µs 126.262µs +0.85%
DROP 98.240µs 101.313µs +3.13%
PUT 126.550µs 129.379µs +2.24%
TAKE 125.602µs 126.872µs +1.01%
EXAMINE 116.760µs 125.031µs +7.08%
INV 80.097µs 90.335µs +12.78%
JUNK 125.240µs 126.209µs +0.77%
LOOK 105.105µs 112.236µs +6.78%
SAY 92.1888µs 102.568µs +11.26%
SNEEZE 90.4013µs 96.355µs +6.59%
$CLEANUP 145.407µs 145.440µs +0.02%
$RESET 107.439µs 117.315µs +9.19%
Average %Change: +5.14%
Not very scientific at all, lots of averages of averages, but it gives me some
idea of the performance change. Which is not very much. I’m guessing that the
slight performance increase maybe because FindAttr cannot inline the Is call?
I’ve been performing a lot of testing and benchmarks on these changes — which
explains me going quiet again. I think all the kinks have been dealt with…
As a bonus, the inefficient Thing.Attrs method has been removed :)
WolfMUD has a very simple locking scheme when it comes to concurrency. Over
time things have become a little muddied with specific locks being added for
certain situations. The attr.Locate attribute springs to mind. Moving logic
into attr.Thing it may now be possible to just lock a Thing and not individual
attributes which would simplify the code a lot. Something I’ll be looking into
next.
These changes and a few other minor ones are now on the public dev branch.
--
Diddymus
Up to Main Index Up to Journal for November, 2019