David 'Bombe' Roden wrote:
> On Sunday 30 July 2006 15:42, David Sowder (Zothar) wrote:
>
>   
>> Perhaps it's a lot more work and harder to do, but I think it would
>> be much better to fix the corner cases with the Hashmaps used by
>> node.getPeerNodeStatusSize() rather than calculating it for every
>> /darknet/ page load.  Part of the idea of
>> node.getPeerNodeStatusSize() was for the information to be both
>> accurate and available to other parts of the node at near zero CPU
>> cost.
>>     
>
> Uhm, I'm not sure I completely understand what you mean but iterating 
> once (!) over an array of 5 to 50 PeerNode objects and calling a method 
> that simply returns an int (which most probably gets inlined anyway) is 
> something I do consider "near zero CPU cost."
>   
Whereas asking a Hashmap it's size probably doesn't iterate over an 
array as it probably tracks it's size with a variable.  While iterating 
over the peers list is not a big deal for the /darknet/ page, it may 
matter if knowing the number of connected peers is useful in one or more 
of the pieces of the node's code that may run tens of times a second or 
more.
> The problem with node.getPeerNodeStatusSize() is that state information 
> for a PeerNode can change during the point getPeerNodeStatusSize() is 
> called and the PeerNode's status itself is shown on the page. To fix 
> that you'd either have to synchronized Node externally (which is a 
> _very_ bad idea) or you'd have to return a list of shallow PeerNode 
> copies that don't change their state after returning them.
>   
The thing you state as being a problem isn't really a problem in my 
mind, though it seems you were fixing a different thing than I thought 
you were fixing.  On the other hand, iterating the list of PeerNodes, 
locking each one, grabbing the information you need, unlocking it and 
counting the statuses is the only way to fix the consistency issue you 
talk about.  Your code does not, for example, fix the case where a 
PeerNode goes into or out of routing backoff and the status displayed 
disagrees.  I personally took the approach that that whole class of 
conditions was not important when it only matters for us silly humans, 
who can reload a page anyway.

What does nag me is, for example, peers that, on the /darknet/ page, 
have a status of disconnected but are clearly connected when looking at 
the other fields in the table, yet nothing changes on repeated page 
loads (i.e. the status updating code is broken somewhere that hasn't 
been found in the several times I've looked).  I think this nagging 
issue is a much bigger deal than the changes your made to the code, 
which, in my experience happened no more than every hundred page loads.

Reply via email to