public inbox for frysk@sourceware.org
 help / color / mirror / Atom feed
* "int pid" replaced with ProcessIdentifier
@ 2008-02-07 15:52 Andrew Cagney
  2008-02-14 11:00 ` Mark Wielaard
  0 siblings, 1 reply; 3+ messages in thread
From: Andrew Cagney @ 2008-02-07 15:52 UTC (permalink / raw)
  To: frysk

FYI,

I've pushed a low-level change so that frysk.sys's process code uses a 
ProcessIdentifier object instead of "int pid"s; there is one 
ProcessIdentifier per "pid".  The change is slowly working its way 
through frysk.proc.live; it won't be taken further.

The motivation is simple, while "int pid" looks small and efficient; it 
turns out that the code using it really needed an object (leaking 
something like <<new Integer(pid)>>) to either log or hash the pid.  
With a proper object both of these cases can be greatly simplified.

Andrew

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: "int pid" replaced with ProcessIdentifier
  2008-02-07 15:52 "int pid" replaced with ProcessIdentifier Andrew Cagney
@ 2008-02-14 11:00 ` Mark Wielaard
  2008-02-14 14:25   ` Andrew Cagney
  0 siblings, 1 reply; 3+ messages in thread
From: Mark Wielaard @ 2008-02-14 11:00 UTC (permalink / raw)
  To: Andrew Cagney; +Cc: frysk

Hi Andrew,

On Thu, 2008-02-07 at 10:51 -0500, Andrew Cagney wrote:
> I've pushed a low-level change so that frysk.sys's process code uses a 
> ProcessIdentifier object instead of "int pid"s; there is one 
> ProcessIdentifier per "pid".  The change is slowly working its way 
> through frysk.proc.live; it won't be taken further.
> 
> The motivation is simple, while "int pid" looks small and efficient; it 
> turns out that the code using it really needed an object (leaking 
> something like <<new Integer(pid)>>) to either log or hash the pid.  
> With a proper object both of these cases can be greatly simplified.

I looked of some of the changes here and it looks somewhat inefficient,
and very verbose, because it basically replaces everything that already
has a pid int with a call to ProcessIdentifierFactory.create(pid). A lot
of these int pids seems to come from Proc.getPid(), which in turn is a
wrapper for ProcId.hashCode() where the hashCode() of a ProcId is indeed
the int representation of the pid of the Proc.

So why not just use ProcId as process identifier for a Proc since that
is already bound to a Proc object? Or the other way around and do away
with ProcId and use ProcessIdentifier instead of ProcId. It seems
confusing to now have 3 ways to identify a process int pid, ProcId and
ProcessIdentifier (4 if you also count TaskId since I saw also code like
ProcessIdentifierFactory.create(this.process.getMainTask().getTid())).

Cheers,

Mark

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: "int pid" replaced with ProcessIdentifier
  2008-02-14 11:00 ` Mark Wielaard
@ 2008-02-14 14:25   ` Andrew Cagney
  0 siblings, 0 replies; 3+ messages in thread
From: Andrew Cagney @ 2008-02-14 14:25 UTC (permalink / raw)
  To: Mark Wielaard; +Cc: frysk

Mark,

You're right, but we've also not finished working through these 
transformations either.  As code gets finished, the hacks you refer to 
will disappear.

There's one thing that is probably worth clarifying though.  Things 
break down as follows:

ProcessIdentifier -> this is a live running host system dependent value.
Like the rest of frysk.sys, beyond frysk.proc.live and test cases, code 
should not be using those interfaces.  A grep, unfortunately, shows 
otherwise.  We've lots of work to do.

Proc/Task -> this is frysk's internal representation of a process/task 
and what client code manipulate

It's important that, as we push towards having our architecture support 
remote targets, we clarify the separation between these two.

Andrew


Mark Wielaard wrote:
> Hi Andrew,
>
> On Thu, 2008-02-07 at 10:51 -0500, Andrew Cagney wrote:
>   
>> I've pushed a low-level change so that frysk.sys's process code uses a 
>> ProcessIdentifier object instead of "int pid"s; there is one 
>> ProcessIdentifier per "pid".  The change is slowly working its way 
>> through frysk.proc.live; it won't be taken further.
>>
>> The motivation is simple, while "int pid" looks small and efficient; it 
>> turns out that the code using it really needed an object (leaking 
>> something like <<new Integer(pid)>>) to either log or hash the pid.  
>> With a proper object both of these cases can be greatly simplified.
>>     
>
> I looked of some of the changes here and it looks somewhat inefficient,
> and very verbose, because it basically replaces everything that already
> has a pid int with a call to ProcessIdentifierFactory.create(pid). A lot
> of these int pids seems to come from Proc.getPid(), which in turn is a
> wrapper for ProcId.hashCode() where the hashCode() of a ProcId is indeed
> the int representation of the pid of the Proc.
>
> So why not just use ProcId as process identifier for a Proc since that
> is already bound to a Proc object? Or the other way around and do away
> with ProcId and use ProcessIdentifier instead of ProcId. It seems
> confusing to now have 3 ways to identify a process int pid, ProcId and
> ProcessIdentifier (4 if you also count TaskId since I saw also code like
> ProcessIdentifierFactory.create(this.process.getMainTask().getTid())).
>
> Cheers,
>
> Mark
>
>   

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2008-02-14 14:25 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2008-02-07 15:52 "int pid" replaced with ProcessIdentifier Andrew Cagney
2008-02-14 11:00 ` Mark Wielaard
2008-02-14 14:25   ` Andrew Cagney

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).