Making KMemoryInfo useful (Re: KDE/kdelibs/kdecore)

Lubos Lunak l.lunak at suse.cz
Sat Mar 20 15:14:17 GMT 2010


 Let's continue only on k-c-d.

On Saturday 20 of March 2010, Pino Toscano wrote:
> Alle sabato 20 marzo 2010, Lubos Lunak ha scritto:
> > On Friday 19 of March 2010, Pino Toscano wrote:
> > > SVN commit 1105258 by pino:

> >  The class, as it stands, is useless for pretty much everybody. Raise
> >  your hand all the none of you who can just look at the class docs
> >  and say what the class actually _really_ tells you. I can't, I had
> >  to make a testapp to figure it out. I think just the fact that
> >  neither I nor the class author can get the class right is enough of
> >  a proof.
>
> Well, there's a difference between saying "the apidocs" is not enough
> and "the class is useless" (which is not the case). I don't think the
> current apidocs is finished nor complete, but honestly I find the above
> a bit too much.

 Sorry. I might be okay with considering the class to be a simple wrapper 
around /proc/meminfo and having it in libkworkspace for kio_sysinfo etc., 
with the docs saying that it is nothing more than a simple wrapper, but this 
is really bad for kdelibs, enough for me to prefer it not being there rather 
than being this way. The class itself is bad, because it encourages wrong 
usage. What will the developer who comes across it think? "So here's this 
KMemoryInfo class that will tell me about memory, I want to know how much 
free memory there is that my app could use, so that's, yeah, 
KMemoryInfo::FreeRam it is" ?

 And I'm really not expecting anybody to do the research to find out what 
those values mean (getting it wrong is sooo easy) and not make any mistake 
somewhere. For example, Okular's DocumentPrivate::getFreeMemory() here 
currently returns 18446744070998462464. That is about 15.99 Exa. I had to 
look the name up, so I'm pretty sure that's not how much I have, in fact the 
computer is right now pretty low on available memory.

 So I strongly object to including a class that makes it easy to shoot oneself 
in the foot. If it's to be there, it has to be at least reasonable if not 
simple and clear. We have already bad enough reputation for memory demands, 
often undeserved just because most people can't get this right, we fret 
whenever Phoronix or whoever publishes another broken measurement and all 
news sites link to it, and now we are to help it?

> >  First of all, it would be nice if the docs said what the reported
> >  unit is. Second, it is essential that it says what the various
> >  memory detail types mean and preferably also how they should be
> >  used.
>
> I can fix these.

 So you know what the difference between buffers and cached is and how it 
matters in practice? I don't, not really.

> >  Finally, since with this class we're basically asking apps to use
> >  memory more aggressively if it's available, it would be wise to also
> >  have a way to free that memory if necessary. We surely don't want to
> >  end up repeatedly running out of memory just because of Gwenview's
> >  image cache or similar. So I suggest the class becomes QObject, gets
> >  a signal for that and refers to it in the documentation (such as in
> >  the FreeRam + ReclaimableRam case), even if it doesn't do anything

 Thinking about this, this is wrong too, it should be just AvailableRam. Who 
cares how much of it the system uses when it'd be otherwise free anyway.

> >  right now. Maybe there could be also a function that'd trigger this
> >  signal, so that apps needing memory could ask others to free their
> >  caches.
>
> Again, I'm not asking applications to use it for improve their memory
> usage,

 What is the use case then? Nobody cares about SharedRam, BuffersRam or 
CachedRam, it actually doesn't matter even for kio_sysinfo or ksysguard, 
unless they want to keep reporting confusing values that have been reported 
by everything since ever.

 You think in terms of improving the situation by moving platform-specific 
code into one place, but it does not justify the resulting problems if this 
ends up as public kdelibs API. KRandom is also KDE API for rand(), but it 
takes care its users do not end up always with the same sequence because they 
forgot to call srand() or did it incorrectly.

> I'm just asking applications to use it instead of manually look 
> themselves reading /proc/meminfo, calling GlobalMemoryStatusEx() or
> whatever the method for the current OS is. What applications do with the
> memory information should be not businness of this class. (And again, I
> can just add that to the apidocs.)

 It is the bussiness of a class if many of its users will use it incorrectly. 
And apidocs is not going to help with that, since if documentation could help 
here, it would have already solved the problem of people getting memory wrong 
all the time.

> >  I object to the inclusion of the class unless it is fixed. It's
> >  enough that almost everybody gets interpreting memory usage wrong
> >  and I really don't want us shipping a little Phoronix directly in
> >  kdelibs.
>
> Just like sysinfo() or /proc/meminfo is not a "little Phoronix", I don't
> think a wrapper around them (on Linux, at least) is a "little Phoronix".

 Oh it is, it exactly is. Phoronix benchmarks are numbers that themselves may 
be correct, but they come with unclear meaning, often confused conclusions, 
and are easy to reach. This is going to be exactly the same.

-- 
 Lubos Lunak
 openSUSE Boosters team, KDE developer
 l.lunak at suse.cz , l.lunak at kde.org




More information about the kde-core-devel mailing list