Matthew Toseland wrote:
> Why can't we just construct the main table before the summary?
>
That's what we were doing before my HashMap stuff was added. Since the
HashMap stuff is currently only used for the periodic log message,
perhaps we can take /darknet/ (and other similar places) back to the
table then summary approach.
> On Tue, Aug 01, 2006 at 08:23:42AM -0500, David Sowder (Zothar) wrote:
>
>> 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.
>>
