On Wed, Oct 02, 2002 at 10:26:32PM +0200, H.Merijn Brand wrote:
> > >  SYNOPSIS
> > >         use Devel::Internals;
> > 
> > A little broad.  Perhaps Devel::Memory?
> 
> My intent was to gather more internal states. Whatever useful.

That's a potentially very large number of exported functions and a very
large interface.  Best to partition it off.

Besides, you can always write Devel::Internals later and plug Devel::Memory
into it.

Also, there's already an Internals.pm


> > >  DESCRIPTION
> > >       sbrk
> > >           returns the current memory top. See sbrk(2) manual page
> > 
> > I realize that sbrk() is a familiar concept to deep C programmers on
> > BSD-style systems, but in order for this to be useful to Perl programmers,
> > or even users not familiar with BSD, you're going to have to name it
> > something more descriptive than sbrk() and explain better what it does.
> 
> That's why there /is/ an Internals.pm: to hide the gory details and
> implementation specifics to the user.

But you're not.  You're just exposing sbrk(), which is a gory detail. My
sbrk man page describes sbrk as being used to find "the current location of
the program break" which means nothing to me.  Nor does "returns the current
memory top".

It'll be even worse on an OS which doesn't have sbrk, which means no sbrk(2)
man page to look at.

Does sbrk() just return the current number of bytes allocated to the
program?


> > unrelated subroutines both calling MemUsed() in the same program.  foo()
> > calls MemUsed() then bar() then foo() calls MemUsed() again expecting to get
> > the memory used since it's last call but it's actually getting the memory
> > used since bar() last called it.
> 
> Bad luck. we don't have any tool to separate.

You should be able to seperate it easily, unless I really don't understand
sbrk() (which is entirely possible).  Here's a sketch implementation of
start(), stop() and used_since_last().

    sub start {
        my $self = shift;
        $self->{start} = $self->sbrk;
        return;
    }
    
    sub stop {
        my $self = shift;
        return $self->sbrk - $self->{start};
    }

    sub used_since_last {
        my $self = shift;
        my $sbrk = $self->sbrk;
        my $used = $sbrk - $self->{last};
        $self->{last} = $sbrk;
        return $used;
    }

Should it be anything significantly more complicated than that?


> > If you're going to keep state like this, better just have an object to
> > represent various views on the memory state.
> 
> No way. This was meant to be a *low* level interface to help you find memory
> hogs and other possible snakes. I see no use whatsoever in any production
> environment. That's why I went for Devel:: namespace

It may not be turned on when the code is run in production, but end-users
will want to use it in their normal day-to-day code.  For that you'll need
something a bit less cryptic and prone to error than the current MemUsed()
implementation with a global state.

If this is just a tool for p5p core hackers, then please don't eat the broad
name Devel::Internals.


> > >           Used in scalar context returns the current sbrk value
> > 
> > Shouldn't sbrk() do that?  Is this the same value as MemUsed() would print
> > in void context?
> 
> Complaints enough already about sbrk (). MemUsed () should hide the
> implementation. That's why.

The MemUsed() docs simply say it returns what sbrk() returns.  You're just
shuffling the complexity around, MemUsed() users still have to understand
what sbrk() does.


> MemUsed ("blah"); # void context
> 
> should print a message to STDERR
> 
> while
> 
> my $mem = MemUsed ("now");    # Scalar context
> 
> should print nothing, but return the sbrk value, hiding the sbrk call to the
> user, but still putting it on the history stack.

Functions which print something in one context and return something in
another are a very bad idea.  You're conflating form and functionality and
mixing in context magic just to confuse the issue some more.


> > >           Used in list context returns the values saved on every call
> > 
> > ?
> 
> See another answer in this thread proposing to return a list of lists. I like
> that.

Seperate out the three different pieces of functionality, returning the
current memory state, printing a memory diagnostic and returning the memory
history, into three different functions.  They are operating on the same set
of data, but doing three different things with it.

Smashing the three together makes the function more complicated, the
documentation more complicated, the testing more complicated and the use
more prone to subtle bugs such as forgetting that foo(MemUsed); should
probably be foo(scalar MemUsed);


-- 

Michael G. Schwern   <[EMAIL PROTECTED]>    http://www.pobox.com/~schwern/
Perl Quality Assurance      <[EMAIL PROTECTED]>         Kwalitee Is Job One

Reply via email to