#417 closed defect (fixed)
Reentrance, Optimization, or both?
| Reported by: | Owned by: | Paul Price | |
|---|---|---|---|
| Priority: | high | Milestone: | |
| Component: | PSLib SDRS | Version: | unspecified |
| Severity: | normal | Keywords: | |
| Cc: |
Description
There are many cases where a psLib or psModule function must allocate dynamic
memory. For instances where speed is crucial, I've been declaring these data
structures static, then allocating them once the first time the function is
called, and then setting the persistent memory bit. However, this causes the
function to no longer be re-entrant. What has higher priority? Some possible
solutions:
1) Allocate the data structure every time; code speed will decrease.
2) Continue current approach.
3) Put a mutex around access to that data structure.
4) other ...
Change History (4)
comment:1 by , 21 years ago
| Resolution: | → fixed |
|---|---|
| Status: | new → closed |
comment:2 by , 21 years ago
Unfortunately, the psAlloc()s are fairly expensive. Each potentially results
in a system call which can take tens of thousands of cycles. I wrote a simple
loop which called psAlloc() repeatedly. The average time for a psAlloc() was
tens of thousands of CPU cycles.
For psMinimizePowell(), that might not be a problem, however, there will
other cases (psStats, and others) where it will be.
comment:3 by , 21 years ago
I've suggested to Gene that we discuss the issue of thread safety in a weekly
meeting, but in the mean time let's assume that static variables are
undesirable, and should only be used when code profiling indicates that they are
necessary.
comment:4 by , 21 years ago
From the SDRS:
Due to current developments in CPU architecture, we must assume that
PSLib will be used in a threaded environment. However, coding the
library to be thread-safe may have implications for the speed of the
library and for the simplicity of use. We therefore make the
following policies:
\begin{itemize}
\item The memory management and error stack functions, defined below
(Sections~\ref{sec:memory} \&~\ref{sec:errors}), must be written to
be thread-safe, since we cannot risk this crucial area being
unstable. However, this can have a large impact on the efficiency
of the code, and so we specify that this behaviour may be activated
and deactivated dynamically. The default behaviour, however, will
be thread-safety, since it is more important to err on the side of
being safe rather than efficient.
\item Re-entrant versions of system calls and external library
functions should be used. We expect that these cases are
sufficiently small that we are prepared to err on the side of
caution.
\item The practise of using \code{static} variable to achieve high
efficiency (e.g., so that subsequent calls do not have to repeat a
large memory allocation) should be kept to an absolute minimum.
Where it has been justified (i.e., through code profiling), the
\code{static} variable must be protected by amutex.
\item Cross-thread synchronization for PSLib's fundamental datatypes
(\code{psArray}, \code{psList}, etc.) is left to the end-user
(although some convenience is provided --- see below).
\end{itemize}
As a convenience to the user in achieving thread-safe operation, each
of the data structures classified as a collection (i.e.,
\code{psList, psHash, psMetadata, psArray, psPixels, psVector,
psBitSet}) and \code{psImage} shall contain a member, \code{void
*lock}, which provides a place for the user to carry around a mutex or
semaphore. This is provided so that the user doesn't have to pass
around both the structure and a mutex, or wrap PSLib structures in
their own thread-safe structures that contain a mutex. PSLib is not
responsible for allocating, setting, checking or freeing the
\code{lock} --- these are entirely the responsibility of the user.
PSLib provides only a place to hang it. Your mileage may vary.
If these policies become a burden on the processing speed, we can
investigate alternative measures, such as defining specifically
re-entrant versions of select speed-critical functions.

I think this is the example in psMinimizePowell you mentioned:
In this particular case, you're setting up a variable before the main compute
happens. That is, you're saving a few cycles by making a variable static, and
then the main compute will involve lots and lots of cycles. That's just not
worth it, and you should eliminate all such uses of static variables.
Same deal with psVectorFitSpline1D, vectorFitPolynomial1DCheby,
psVectorFitPolynomial1D, p_psDetermineBracket2, p_psLineMin.
If the allocation is in an inner loop, then we can start thinking about ways
around it, but even then I'd like to avoid the use of a static variable.
Remember, for every static variable allocation, you need to have a corresponding
free in psLibFinalize.
The implementation of buildSums1D looks fine:
static void buildSums1D(psF64 x,
{
}
In this case, you allocate the vector if you need to, but most of the time
you're only performing the computation --- looks good, and there's no need to
make anything static. (Though I'd put in an "else" before the second "if".)