[systemd-devel] [PATCH] bootchart: use conf-parser & CamelCase names in .conf

[Thomas H.P. Andersen]

On Thu, Feb 14, 2013 at 1:37 AM, Kok, Auke-jan H
<auke-jan.h.kok at intel.com> wrote:
> On Wed, Feb 13, 2013 at 4:00 PM, Kay Sievers <kay at vrfy.org> wrote:
>> On Thu, Feb 14, 2013 at 12:42 AM, Kok, Auke-jan H
>> <auke-jan.h.kok at intel.com> wrote:
>>> On Wed, Feb 13, 2013 at 3:32 PM, Lennart Poettering
>>> <lennart at poettering.net> wrote:
>>>> On Wed, 13.02.13 14:27, Kok, Auke-jan H (auke-jan.h.kok at intel.com) wrote:
>>>>
>>>>> > Hmm, what does this stand for? Wikipedia doesn't have it, can't be that
>>>>> > well known...
>>>>>
>>>>> PSS is the alternative to RSS... You probably won't find an
>>>>> explanation anywhere else but the kernel source code:
>>>>>
>>>>> Documentation/filesystems/proc.txt:
>>>>> =====
>>>>> The /proc/PID/smaps is an extension based on maps, showing the memory
>>>>> consumption for each of the process's mappings. For each of mappings there
>>>>> is a series of lines such as the following:
>>>>>
>>>>> 08048000-080bc000 r-xp 00000000 03:02 13130      /bin/bash
>>>>> Size:               1084 kB
>>>>> Rss:                 892 kB
>>>>> Pss:                 374 kB
>>>>>
>>>>> [...]
>>>>>
>>>>> The first of these lines shows the same information as is displayed for the
>>>>> mapping in /proc/PID/maps.  The remaining lines show the size of the mapping
>>>>> (size), the amount of the mapping that is currently resident in RAM (RSS), the
>>>>> process' proportional share of this mapping (PSS),
>>>>> =====
>>>>>
>>>>> so, PSS translates to "proportional share of the mapping(size) that is
>>>>> resident in RAM"
>>>>>
>>>>> PSS will do fine, I suppose :^)
>>>>
>>>> RSS is an acronym for "Residential Set Size". PSS for
>>>> "Propertional Set Size". Hence the option for bootchart should be
>>>> "ProportionalSetSize="?
>>>>
>>>> What does the option actually do? Do we actually need the option? If
>>>> not, we might just drop this source of confusion? And we do need it,
>>>> maybe make it explanatory as int "PlotProportionalSetSize=" or so?
>>>
>>> When enabled, it creates an additional graph (just like the entropy
>>> option, or, if you have booted with initcall_debug) that plots the PSS
>>> for each process.
>>>
>>> It's a highly usable graph for people working on systems with less
>>> memory, so, I'd like to keep it.
>>>
>>> Example of how it looks here:
>>>
>>>     http://foo-projects.org/~sofar/bootchart-20120401-0710.svg
>>>
>>> Plotting of PSS is disabled by default since it has quite a
>>> performance impact (it requires parsing /proc/<NN>/smaps for each
>>> process, which can be hundreds of kilobytes large each).
>>
>> That looks nice, yeah.
>>
>> But shouldn't it just be called PlotMemoryUsage= or something instead
>> of the using the "algorithm name" in the config switch to enable it?
>
> Right, that's totally fine with me, really. I suck at naming things ;^)
>
> EntropyGraph... similarly thus.

So, can I commit the patch tonight with these names:

Rel => Relative
Freq => Frequency
Entropy => PlotEntropyGraph
Pss => PlotMemoryUsage

and parsing these as bool so the .conf file will have e.g.
"PlotMemoryUsage=no" ?

What about the options from the cli? Should we keep them as e.g.
"--freq" for backwards compatibility or make similar changes there?