Re: [PATCH] SMP Cache Manager, Phase2

On 10/14/2010 07:20 AM, Amos Jeffries wrote:
> On 14/10/10 18:09, Alex Rousskov wrote:
>> SMP Cache Manager, Phase2 implementation.
>>
> <snip>
>>
>> More polishing left for future projects: Move CacheManager to Mgr
>> namespace and src/mgr/ directory. Use SBuf instead of String for
>> ActionParams and TypedMsgHdr. Rename Ipc::TypedMsgHdr to Ipc::Msg,
>> Ipc::SocketMsg, or similar because it maintains more than just msghdr
>> struct. Fix #include order in old source files affected by the
>> Mgr::RegisterAction change. More action aggregation and Phase3 changes.
>
>
> Thank you.
>
>
> Audit Checks:
> * please make sure all the new mgr/*.h in the include lists are not
> adding regressions. There seem to be a fair few added which are merely
> mussing up otherwise find lists.

I assume you are talking about the alphabetical order of old #include
directives. I will fix all the affected old files as a separate commit.
The newly added files should have the "right" order of includes already.

> * Why double across the board for all the counters?

I am not sure "double" was the right choice, but here is the rationale:

- 32bit counters used in the old code cannot be used with SMP because
they are likely to overflow as we merge results from multiple kids.

- Even some 64bit counters (e.g., those counting small objects like
bytes) might overflow when many kids are merged.

- Using two 64bit counters (gb_*) seems like an overkill here.

- Using "double" counters is simple and will work forever. Many of the
counters are not reported raw but as rates or fractions anyway.

We should have used a typedef for this. Sigh.

> does it accurately portray TB counts to the byte for large boxes?

If we use doubles, I do _not_ think huge values will be accurate to the
byte. Is it really important for something like a 3 terabyte value to be
accurate to the last byte?

If it is important, what do you suggest? 64bit counters? A pair of 64bit
counters? A custom decision for every counter??

> * don't forget to update SourceLayout with the src/mgr/ progress :)

Done.

Thank you,

Alex.
Received on Thu Oct 14 2010 - 17:12:09 MDT