BTimeSource should be cleaned up

[Barrett]

BTimeSource looks unfinished and it's code is in some parts messy.

[Barrett]

Other than making the SlaveNodes members private, i've replaced the current array implementation with a list based one. This is because, the way it was done hadn't any performance improvement over a list. In fact we had an insertion time which was only in the best case O(numOfSlaves) but in the worst case it was O(MAX_SLAVE_COUNT), the same consideration apply for removal. Insertion cost is now constant. Where removal is in the worst case O(numOfSlaves).

My initial plan was to remove the SLAVE_NODES_COUNT as it doesn't have any sense now, but thinking of it something that should limit the max number of nodes i've transformed it into the MAX_SLAVE_COUNT constant, which is checked when a new slave node is added. I need some expert's opinion before to remove it.

The patch contains also a lot of cleanup to make the class more readable, and make way to fix in future the 'XXX' warning from Marcus about it not deleting crashed nodes. I'm investigating also about it, in which way node crashed should be detected? Is it ok to look at the port and see if it's already working?

[Barrett]

Compared to the previous patch this add better error checking when adding/removing a slave node.

[Barrett]

Updated patch with just two little fixes.

[Barrett]

I think the patch is ok now, and i've tried to make clear what my modifies are.

[korli]

Patches 1 & 2 are OK. Patch 3:

• the use of GetNext() requires Rewind() before each loop.
• "Author" => "Authors"
• authors are listed in lexicographic order by last name.

[Barrett]

Thanks for helping in getting my code included into Haiku.

[korli]

Applied in hrev48916.

[Barrett]

I have reflected a bit about the possible solution of the "XXX" things in BTimeSource which it seems the last thing to do to close this ticket, and i have the following plans. Actually, BMediaRoster and friends are writing to the node port to get it's timesource. The problem is that the node has crashed at this point and in the media_server there's not trace of it's timesource. I'm planning to resolve it by adding a member to the registered_node struct to allow to trace the node timesource. This imply to update this field every time the BTimeSource change.

We already have some facilities to detect when a team crashed (CleanupTeam()) and in this place i would like to add the code to query the time source (if it's already up) and remove the crashed node.

[Barrett]

I've implemented the plan in the previous post, i've pushed it in branch on github :

​https://github.com/Barrett17/haiku/commits/btimesource_cleanup

Feel free to comment out.

[Barrett]

Is there some interest in having it included into Haiku?

[axeld]

AFAICT, sure, but I'm a bit rusty when it comes to the Media Kit, unfortunately.

[Barrett]

For me it's a joke. I might not have considered that the whole project is a joke. Beware that to keep you running, you need a constant acceleration. I'm stopping to contribute anything, i will set on the arguing-about-defects mode, instead to fix them myself.

[bonefish]

Come on, you know what the situation is like. It isn't like there's a horde of people sitting around, just being too lazy to look at your patches. There probably isn't more than a handful of committers who have enough working knowledge of media kit internals to be comfortable enough to review your changes, most probably being "a bit rusty" as axeld put it and/or having very little Haiku time these days. I know this applies to me and I won't even consider investing probably a few hours of time that I simply do not have.

It also doesn't help to circumvent the patch contribution process. Yes, GitHub is more convenient for commenting on patches and if you have your code up there already, it is also less work for you, but the Haiku's current process is to attach patches to tickets.

Anyway, I guess the easiest solution would be to grant you commit access. Since I don't recall having reviewed any of your patches, I won't make such a proposal (or even vote on it), but maybe someone who has, will do so. You can also ask yourself on the development list.

[Barrett]

It is not something personal or related to being lazy, but in general it isn't offered any alternative to that. I just don't see an affordable compromise even if there's not general consensus for commit access. I know the situation, but there are unexplained things such as me not being in the contributors list, except for Italian translations work which is really the least thing i have done for the Haiku project. Not everything can be in control and it's not really not guilt of anyone, but i have not been here to annoy people. So i may not be a lot kindly sometimes, but at some point it's licit the question if all that makes sense. If i cannot contribute easily (for both committer and author) so it's time to stop it. The reason for having them in github is that in pratice managing all those patches is impossible in trac, at least for me, and the result is a lot of unneeded noise. Since those are changes which should be discussed, i don't see also a pratical way to do that, like it's done in some unmerged branches appearing in the commit list.

[kallisti5]

Replying to Barrett:

It is not something personal or related to being lazy, but in general it isn't offered any alternative to that. I just don't see an affordable compromise even if there's not general consensus for commit access. I know the situation, but there are unexplained things such as me not being in the contributors list, except for Italian translations work which is really the least thing i have done for the Haiku project. Not everything can be in control and it's not really not guilt of anyone, but i have not been here to annoy people. So i may not be a lot kindly sometimes, but at some point it's licit the question if all that makes sense. If i cannot contribute easily (for both committer and author) so it's time to stop it. The reason for having them in github is that in pratice managing all those patches is impossible in trac, at least for me, and the result is a lot of unneeded noise. Since those are changes which should be discussed, i don't see also a pratical way to do that, like it's done in some unmerged branches appearing in the commit list.

We appreciate all the work you've done! The general process to getting commit access is as follows:

• Be around and contribute to the ML discussions
• Begin to look at bugs/enhancements and submit patches through trac
• As you begin to work with commiters they nominate you for commit access listing your work / patches in trac tickets
• The commiter community votes on haiku-development

If this isn't on our site it should be.

Either way, the vote is going well.

If the commiter community approves you I'll add you to the maintainers list. (better late than never! :-)) If for some reason you don't get enough votes I'll add you to the contributors list.

Barrett, what's your full name so I can add it?

[Barrett]

I think i am too late for the name question :-) For what i can, once i get comfortable with, i will help to review patches. And sorry for being a bit rude, but it was helpful in the end.

[Barrett]

Fixed in hrev49384.