[haiku-gsoc] Re: XSI semaphore implamentation patch #2
- From: Ingo Weinhold <ingo_weinhold@xxxxxx>
- To: haiku-gsoc@xxxxxxxxxxxxx
- Date: Wed, 30 Jul 2008 00:15:13 +0200
On 2008-07-29 at 16:26:52 [+0200], Salvatore Benedetto <emitrax@xxxxxxxxx>
wrote:
> 2008/7/29 Ingo Weinhold <ingo_weinhold@xxxxxx>:
> >
> > On 2008-07-29 at 00:24:29 [+0200], Salvatore Benedetto <emitrax@xxxxxxxxx>
> > wrote:
> >>
> >> attached is a second patch of xsi semaphore implementation.
> >> It includes the SEM_UNDO features.
> >>
> >> I actually spent more time than I though on this one, because
> >> the sem_undo list get accesed not only when a process exit,
> >> thus executing all its sem_undo request, but also when calling
> >> semctl with SETVAL or SETALL on a semaphore (all sem_undo request
> >> must be cleared, that is deleted).
> >>
> >> I reworked it three times in order to keep it as simple as possible
> >> and eventually came up with a single global sem_undo list (sUndoList).
> >> I wanted to add as little as possible to the team structure.
> >>
> >> The complexity of the algorithm when removing an element is of course
> >> O(n)
> >
> > As are xsi_sem_undo() and IPC_RMID (or would be, if it removed the undo
> > values).
> >
> >> as I iterated through the list but only a simple short value has been
> >> to the team
> >> structure (at first it was a boolean value) and speed shouldn't really
> >> be a problem
> >> as I think we won't have many semaphores in the systems.
> >
> > Er, and you didn't wonder whether trading performance for a few bytes per
> > team that uses XSI semaphores might not be the best idea?
>
> Nope. Because it happens only on process exit.
>
> >
> >> Anyway, if it happens to be a problem, I'll try with another solution.
> >
> > Well, I proposed one that doesn't have the performance issues. I wonder
> > why
> > you even asked.
>
> Because as I said, the performance issues happens only on process exit and
> from
> an user point of view it shouldn't be a problem.
Not sure what to say. You wrote yourself in your previous mail that SETVAL
and SETALL are affected (BTW, the latter is even very suboptimally
implemented and thus O(m*n) with m = number of sems in the set and n = total
number of undo entries). And as I wrote in my previous mail RMID should be
affected in the same way. So the performance issues when a lot of semaphores
are used are definitely not limited to program exit only.
> >> The code has been tested with bonnie++, which uses the SEM_UNDO feature,
> >> and
> >> it seems to works fine.
> >>
> >> I've also fixed some bugs that weren't easy to discover by simple
> >> reviewing the code
> >> in the first patch and bonnie++ helped with this. In fact it now runs
> >> in multiprocesses mode.
> >
> > Very nice. I hope the insight that code review won't uproot all bugs
> > encourages you to actually write unit tests.
>
> I'd actually prefer to reuse some other test like the one I've attached in
> the previous email,
That's fine, if you find enough for a reasonable coverage.
> or use some real programs like bonnie++.
"Real programs" are definitely not a good substitute for unit tests, since
they usually use only a small fraction of the API and possible test cases.
Please understand that others have to work with the implementation you
produce. Particularly people porting applications. And there are few things
that are less fun than unreliably occurring problems with code one is not
familiar with.
CU, Ingo
Other related posts:
