wiki:WeeklyMinutes20110412

BIND 10 Team Meeting

2011-04-12

Attendees

Kambe
Likun
Stephen
Aharen
Tomek
Shane
Jeremy
Fujiwara
Jerry
Ocean
Jelte
Jinmei
Michal

Call Time

Shane: We agreed to do sprint planning at 14:30 UTC.

Stephen: Either is okay for me.

Jinmei: Later is always better for me. Especially if I am going to join the daily call before the weekly call. If others want to start it earlier then I am okay with that, but I will probably skip the daily call immediately before the bi-weekly call.

Jelte: I have no opinion either.

Michal: If there are different times, I would like an e-mail saying which time this call is happening.

Likun: It's okay.

Shane: Any preferences from JPRS?

Kambe: It's okay.

Shane: We'll have them at different times, and I'll send a mail every week.

Protocol for closing bug tickets

We need to insure that we actually solve the problem before declaring it fixed.

Jeremy: This is about ticket #615, but this is about multiple tickets. I had created 7 loadzone tickets and they were all closed without me reviewing them. First I had to remember which tickets, and then when I tested them some of them weren't actually solved.

Jeremy: If someone opens a bug report, the person who opens the bug report should be involved in declaring it fixed. We need a difference between a task and a bug and who should follow up on it.

Michal: Regarding #615... it stayed there for some time asking if it worked... if there are specific points could you re-open it or something?

Jelte: That's a specific case...

Jelte: One possible way forward is to add a state after reviewing, "verify" or something.

Stephen: Or we could get to the point in reviewing to put it back to the bug submitter for review. But only if the submitter is someone in the BIND 10 team, otherwise we have to wait for someone outside the team.

Jelte: In that case we could just close it and add a comment "if this is not fixed please re-open".

Stephen: How easy is it to add states to the Trac system?

Jeremy: It's reasonably easy. But I think it comes more cluttered as we add more things I guess.

Stephen: I'm thinking of a state called "sign-off". So if it is a bug it has to go through a "sign-off" state before we close it.

Jelte: Should we merge it before that?

Michal: Yes, otherwise we can't test it. I think it is more important if someone is not part of the team.

Stephen: I'm worried if someone does not respond to an e-mail. They might have just noticed it - they don't want to spend time. We could end up with a lot of tickets just hanging around.

Shane: Other ticket systems have done things like "this will be closed in 2 weeks if we don't hear back".

Stephen: That adds extra mangement.

Jinmei: We should separate ticket management and the relationship with the reporter. We use the ticket system as a way to mange our progress. We should not wait to close it. It is important to provide feedback to the original reporter. It is not bad to close the ticket while telling the original reporter "we think it's fixed if not just tell us". It's not so uncommon actually. In that sense I think #615 is a very special case because everyone is in the group, so we tend to do it in a very unsocialized way.

Shane: I think that's the right way. We can just send an e-mail when you close a bug ticket.

Jinmei: We include the reporter in the Cc: list of a ticket.

Shane: I think you do get notified, but it's not a very friendly e-mail.

Jinmei: On top of that I think you should do a more humanized action. My question is whether we need a written rule for that. Either way is okay for me.

Shane: I think it will be helpful for people coming on the project, and for people not in the project at all, if it is written down.

Jinmei: I have no problem with that. There may need to be a way to check that process.

Shane: I agree but I'm not sure what that would be.

Jinmei: Maybe just include a list of what reviewer is expected to do.

Shane: You mean someone who receives a bug report?

Jinmei: It's not very formal, but if we get a bug report and we open a ticket, someone fixes it, and when it gets reviewed the reviewer will kindly notify the developer that he or she should notify the reporter.

Jinmei: This is not a suggestion, but in the case of BIND 9 we do not explicitly provide feedback. The Rt just sends the automatic e-mail to the reporter telling them that we believe the problem is solved or something. The minimum level works somehow, but I personally think it could be a bit more friendly. Maybe just having the safety net is not so bad. If we just try to be a bit more friendly as a best-effort basis then maybe it's just enough.

Jeremy: Lets say I have 50 bug tickets (which is a huge amount) and they all start getting closed - and hopefully all fixed. Because they are all closed I lose a way to check them myself. I have no incentive...

Jeremy: As another example - we've had a few cases with regression. The reviewer and the original developer never noticed them, so later on when someone finds the code they don't see that the regression correllates with the ticket that has just closed. But if I had tested it first then the ticket would be still open.

Jelte: But with only 24 hours in a day we probably cannot have you check every ticket before we close it.

Jeremy: But if a ticket is closed then usually people will ignore it unless they have time at that minute.

Jelte: If it is someone on the team I have no problem assigning it to them.

Jeremy: Merge first is fine - as long as it has been reviewed.

Stephen: If we go back into the sign-off state, instead of moving a bug to "closed" we move it to "sign-off", then at the end of the sprint we can review the bugs and move them to "closed".

Stephen: I say, if we do the "sign-off" state then the number of bugs we fix will not be large. At the sprint planning we can go through all tasks in the "sign-off' state and see if it needs to be tested or whatever.

Shane: My concern is making it too complicated. If people want the "sign-off" state then I'm not opposed to it I guess.

Jinmei: Explain.

Stephen: When a bug ticket is done, instead of moving to "closed" you change it to "sign-off". And so at the next sprint planning session we will go through all of the tickets that are in the "sign-off" stage whether we need to take it further or whether we can close it at that point.

Jinmei: I am not opposed, but I am afraid it is too complicated for the expected benefit.

Stephen: If we don't somehow flag the bugs we've tackled then it is difficult for Jeremy to recognize the ones he has to re-test just looking at the list of closed tickets.

Jeremy: Maybe a new state is not needed, we just need to have a policy or procedure. If there was a bug reporter then quickly see if that person was involved or not. That way we don't have to wait to actually close it. In most situations it's okay to close.

Stephen: If we leave it in open state... the person could be on the other side of the world, the ticket gets left open. You're waiting for the response. We end up with lots of tickets open. We can also say "anything more than 2 weeks we close at our sprint planning".

Shane: Okay, for now we'll just have an e-mail to the reporter. This means we need a page on the Wiki explaining this, and also a change to the review policy so that the reviewer will remind the bug fixer to send the e-mail. We can revisit the idea of "sign-off" state later.

When to optimize

See ticket #536 for an example of a specific optimization that may or may not be helpful. http://bind10.isc.org/ticket/536

Jinmei: My point was not so generic. I sent an e-mail.

https://lists.isc.org/pipermail/bind10-dev/2011-April/002152.html

Jinmei: #536 updates the internal data structure from STL to C-style library. It optimizes for the name compression case, but that needs to be revised anyway, so we should at least wait until that point. Even if this is true, I don't request we cancel but I propose we revisit after we have optimized the server performance.

Shane: Does STL really not have a way to resize a structure?

Jinmei: It can, and it can automatically extend.

Michal: It's not about the resizing. I found out that the operator[] is not inlined, so I removed the vector to somehow make the function even smaller and provide the compiler a way to inline it. It took 25% of runtime and is a small function.

Stephen: It depends on the implementation of STL. Different implementations may have their own versions. Certainly things like strings are very much up to the author of the library.

Michal: It doesn't inline anyway because it gave only a 4% improvement to code. When it was written I put it into review because 4% is better than nothing. I agree there are more complications in it, but they are localized - closed in the class. So I don't think it's that problematic. If it really did inline and give us 25% speed up in 5 hours work it would be nice, which is why I did the ticket. I thought it could be an easy way to gain a lot of performance. I agree we need a more higher-level optimization of name compression and so on, but it takes more time to write it.

Stephen: We should not discard performance was we're writting the code. As you're writing the code choosing an efficient way of doing it is important. I think the problem we're going to find is that there's going to be nothing that jumps out. It's going to be lots of little things that add up to a significant performance hit.

Shane: Possible...

Stephen: For example we use Boost shared pointers, even when we don't need to. Sometimes we can get buy with a scoped pointer and sometimes even a raw pointer. With C++ we're making quite liberal use of dynamic memory allocation. That will be a bottleneck - if we can get rid of as much dynamic memory allocation as we can that's only going to be good.

Michal: I kind of agree with Stephen, but it sounds like going through all of the code and combing it line by line...

Stephen: That's why I'd much rather we do it "right" the first time. I've fallen into the trap thinking "why did I dynamically allocate the array there? why not declare it automatically on the stack?"

Jinmei: I don't understand. Are you still talking about shared pointers?

Stephen: No, as a general thing, look at what you're doing and try to optimize it.

Jinmei: In general I think that's taken care of in the process of review. Reviewers often make a comment about the performance. Of course the developer is expected to consider it in the first place.

Michal: If we can choose between multiple ways to write it, and they are approximately the same complexity, then it is always better to chose the faster way.

Stephen: I get the feeling that in avoiding premature optimization we are not thinking about optimization when we're writing the code.

Michal: If it is complicated, I don't write it right away and put a TODO note.

Jinmei: For me it is all a balance between premature optimization and unnecessary overhed in the code. Sometimes we are not aware of the balance. My original intent in this thread is more about premature optimization. Even though we tend to introduce micro-level optimization at the cost of making the code complicated we should generally wait further for that type of optimizations until we do the higher-level structural optimization.

Michal: I agree. In this case it was a special attempt. And since it was already written it seemed pointless to throw it away.

Jinmei: I agree, but leaving complicated code is another risk which is why I would request revisiting it after we optimize further.

Other Topics

Jinmei: We were going to talk about Windows, and maybe look at the face to face minutes for additional topics.

Shane: Okay, we will discuss in 2 weeks.

Last modified 7 years ago Last modified on Apr 12, 2011, 4:01:59 PM