Re: Small patch to show only memory usage.

[Tim Janik <timj gtk org>]

On Sun, 4 Feb 2001, R. Bernstein wrote:

> However I do not agree with the attitude that seems to be put forth
> here that you can slap the user around with gratuitous
> incompatibility. g_mem_profile() has probably been around for several
> years now (what 3 or 4?) and people may be using it and I don't find
> it a compelling reason for breaking it for these people because you
> don't want to use one more name. You exaggerate a bit by writing "x
> functions" when "x" is in fact 1 more, and the additional function is a
> generalization of the existing one -- it is not exactly the same as
> the existing function.

let me put that in perspective. we don't generally carry an attitude of
slapping users with gratuitous incompatibilities. in fact, we usually
take great care to preserve source compatibility where reasonable.
however, things are a bit different for g_mem_profile() in particular,
users could never use that functions without special configure options
upon glib compilation, and the profiling code has significantly changed
already so that users have to adapt their source anyways if they still
want to use profiling features, because g_mem_profile() is essentially
useless without a prior g_mem_set_vtable (glib_mem_profiler_table).
the already introduced source incompatibility and fact that g_mem_profile()
is only interesting during the development phase of a program where an
additional argument to g_mem_profile() is easily added makes me believe
that this particular breakage shouldn't be considered a huge burden.
plus, developers that do make use of g_mem_profile() and don't care to
read up in the Changes-2.0 file will get prompted by their compiler to
figure that things have significantly changed in that area.

> Also, as probably the greatest user of this patch, I can say that I
> find the parameter to log a message useful and convenient, and perhaps
> suggestive of something helpful.  Originally my code did have those
> calls to the log routine beforehand -- it was a bit of a pain.  Calls
> to track memory are often added, removed and moved around or
> conditionally added. Putting everything in one call is convenient.  If
> you don't want a message, put NULL as the message parameter and no
> message will appear.
> 
> And finally as again probably the biggest user of this stuff, I can
> say that I'm not sure every time I want to show allocation pools I
> also want to show large allocations. For convenience, one could create
> a bitmask for these two e.g. G_MEM_SHOW_ALLOCS = GMEM_SHOW_LARGE_ALLOC
> | G_MEM_SHOW_ALLOC_SIZE.  By *forcing* these two to be combined
> programmers can no longer easily choose. In all of the patches I
> submitted, I give the programmer a choice.
> 
> It may be more acceptable to slap around a patch submitter -- although
> you'll find that you'll probably get fewer people willing to
> help.

it has not been my intention to slap you around as a patch submitter.
if with slapping you refer to the API/implementation changes i asked
for, well, that's usual discussion between developers that i wouldn't
want to miss, regardless of me being a patch submitter, the maintainer
supposed to accept a patch, or a third person commenting on someone
elses patch.
if instead you refer to the other group of comments, things like diff
format or code formatting, i can just say, unified diffs are a lot
easier to read, so much even, that i've already rejected context
diffs that were hard to read because the intended changes weren't
apparant anymore.
yes, i could always try to dig up the correct set of source files
that the patch submitter applied his changes to and did a context diff
on, then apply his patch and rediff in unified form to read over it.
for me personally i made the coice to not always go through that
hassle, mainly because
1) it's easy for patch submitters to add "-u" upon their diff
   invocation
2) it can be quite hard in times to figure the correct CVS/tarball
   version, especially if multiple files are involved
3) it can consume a major amount of my time if i have to do that
   for a great portion of patches that i need to consider
4) it significantly increases time spent on composing patch commentary
   emails
so i simply don't take context diffs, though i occasionally may still
coment on them. a choice i'm not going to apologize for, but i hope
you understand the resons now.

> However when you making things more difficult for the
> programmers (by making incompatible changes, by reducing choices, 
> and by rejecting useful conveniences), I have to take objection. We
> are now talking about more people here.

> Finally, all the code is and always have been right there in the
> open. I'm not sure what the benefit of making suggestion and then
> suggestion on top of that when the code's right there for you or
> anyone else to change. It just delays things. The delay doesn't hurt
> me, since I already have code that works and has been tested and suits
> my purpose, although I have to say I wish the GNOME/GTK/GLIB didn't
> have the memory leaks it has to the point where I find myself having
> to use. Maybe the delay which may contribute to undiscovered memory
> leaks is just more of the "slap the end-user around" mentality.

i think you're mistaking glib's caching behaviour with regards to list
nodes and simialr strcutures as memory leaks.

> Again this stuff is in ftp://ftp.gtk.org/incoming/gtk-rocky-010304-0.tar.gz
> as the README and FAQ on www.gtk.org suggest is what should be done. 

ok, stupid me, i only now recognize that your patch actually applies
against the stable branch of glib, while i thought it'd tweak the
newly introduced memory profiler in the CVS version.

i'm sorry for not catching that earlier, but for the stable branch,
API changes are only accepted if:
1) they don't break binary compatibility (which your additiona of
   g_mem_summarize() didn't do)
2) the breakage in interface compatibility is a substantial requirement
   to fix current behaviour or a lack of functionality (which i dare
   saying, doesn't apply to your patch)

(of course, your above comments on keeping source compatibility make
much more sense to me now, and they are prefetly valid concerns within
a stable branch)

---
ciaoTJ