[haiku-commits] Re: haiku: hrev48664 - in src/add-ons/kernel: file_cache network/stack src
- From: Adrien Destugues <pulkomandy@xxxxxxxxx>
- To: haiku-commits@xxxxxxxxxxxxx
- Date: Mon, 12 Jan 2015 19:02:36 +0100
On Mon, Jan 12, 2015 at 12:37:34PM +0100, Stephan Aßmus wrote:
> Hi,
>
> does everyone consider it a good idea to make these refactorings? I thought
> the regressions after the BMessage refactoring and the previous hash table
> refactorings indicate that it's not a good idea with regard to stabilizing
> the code-base before Beta 1. These kernel regressions don't make me feel
> confident. I hope there are not still some in there which are just less
> likely to be noticed, and then they are noticed much later, when no one
> thinks about these refactorings anymore.
Hi,
I prefer doing these now so they get a lot of testing in beta1. This way
it is more likely that any issues they may introduce is found and fixed.
Fear of modifying code because 'it might break things' is a great way to
let the code bitrot. The BMessage refactorings triggered regressiosn
because they exposed bad API design (2 methods with the same name, doing
different things) and an API limitation in the 64-bit version (where
using the appropriate size constant for "maximal size" will trigger an
overflow). The khash changes introduced bugs because khash makes the
code less readable and I missed some subtleties. Replacing it with
BOpenHashTable makes the code easier to read, understand, and maintain.
I think in the long term this is the right thing to do.
It would be great to have a more complete testsuite, but for this we
need to setup a buildbot running haiku so the tests can be run, and/or get
our automated unit tests to run on the libbe_test platform so the other
build bots can run the tests. Without an automated test suite, we can
only rely on ourselves and our users to manually test that the image
runs, and that everything works as expected. This is time-consuming for
everyone, but the fix is not stopping to make changes to the code.
I just pushed another set of BOpenHashTable changes. I hope I did
everything more correctly this time (my own machine still runs, but
that's all I can test...). The last remaining use of khash is in the
filesystem code (the block cache and the rootfs). There is a bit more
work there because of interaction with userlandfs, which currently
provides an userland replacement of khash. I will try to make it do the
same for BOpenHashTable.
--
Adrien.
Other related posts:
