[sldev] Key areas for code review.

Alan Grimes agrimes at speakeasy.net
Sat May 19 18:42:49 PDT 2007


Some people seem to find my posts either interesting or useful so I'll
continue writing them. Other people seem to think I'm a total ignoramus
who needs to have studied this codebase for 3 years before making any
useful observations.

I've been studying the generated inheritance diagram from kdevelop to
figure out which are the most critical classes.

Most parent classes have 3-5 children.

however, there are several major families of classes in SL.

LLIOPipe has roughly 30 children.
LLMotion has about 20.
LLInventoryCollectFunctior has roughly 25 children.
LLHTTPNode has about 25 children.

Curiously, there seems to be two versions of the garbage collector,
LLThreadSafeRefCount and LLRefCount... While it could be argued that a
fast non-reenterant garbage collector is useful for the tighter loops, I
would argue that there should be only one reentrant garbage collector...
Better yet, I'd like to see one of the several popular garbage collector
add-ons for C++ be used here instead of custom (slow, buggy) code.

Predictably, the heirarchy of the script compiler is quite extensive.

The real star attraction is LLView, it is not a root, but a child of
several other extensively used classes. It is critically important
though because fully two thirds of the classes in the entire second life
client are children of this one class. There are, in fact, five
generations of children below LLView. The code itself doesn't look to
good. For example, the public, private, and protected sections are split
and mixed among each other in the class definition. Many const
parameters and methods aren't marked as such (prevents certain compiler
optimizations and automated bug-discovery). And the class seems to be in
need of refactoring because it seems to have multiple responsibilities
that aren't directly related to it's main mission. There are also at
least a few variables which are hardly even used. Considering the
massive number of classes below this class, that is very bad. Each byte
in LLView could mean many killobytes of essentially useless extra
storage space in the running application.


-- 
Opera: Sing it loud! :o(  )>-<


More information about the SLDev mailing list