sim: make Python Root object a singleton

Information
Submitter:	Steve Reinhardt
Repository:	gem5
Branch:
Bugs:
Depends On:
Reviewers
Groups:	Default
People:

Description

sim: make Python Root object a singleton
Enforce that the Python Root SimObject is instantiated only
once.  The C++ Root object already panics if more than one is
created.  This change avoids the need to track what the root
object is, since it's available from Root.getInstance() (if it
exists).  It's now redundant to have the user pass the root
object to functions like instantiate(), checkpoint(), and
restoreCheckpoint(), so that arg is gone.  Users who use
configs/common/Simulate.py should not notice.

Testing Done

src/python/m5/simulate.py (Diff revision 1)

OK, I got ahead of myself... this doesn't compile.  I may redo this whole optional argument scheme using *posargs after all.

Diff:

Revision 2 (+90 -10)

Show changes

	configs/common/Simulation.py
	configs/example/memtest.py
	configs/example/memtest-ruby.py
	configs/example/rubytest.py
	configs/splash2/cluster.py
	configs/splash2/run.py
	src/python/m5/simulate.py
	src/python/m5/util/__init__.py
	2 more

Diff:

Revision 3 (+97 -17)

Show changes

	configs/common/Simulation.py
	configs/example/memtest.py
	configs/example/memtest-ruby.py
	configs/example/rubytest.py
	configs/splash2/cluster.py
	configs/splash2/run.py
	src/python/m5/simulate.py
	src/python/m5/util/__init__.py
	2 more

src/python/m5/simulate.py (Diff revision 3)

You say *posargs in this code, whereas we use *args everywhere else in the python code.  (Python itself uses *args, so I'd suggest a change for consistency).
I assume that the *args is for backward compat where people say instantiate(root).  If so, perhaps you should document this.  Actually, is it really even worth doing all of this?  It's pretty trivial to update a script to support this and if people are updating to the latest M5, it's not a huge change to ask of them.  We don't typically dirty the code with backward compat stuff.

Steve Reinhardt June 28, 2010, noon (June 28, 2010, noon)

Yea, I ended up whacking all of this about two csets later in another patch, so I'm guessing for most people the window of backward compatibility will be the few milliseconds between pulling those csets.  So you're probably right, I should just not bother.

Nathan Binkert June 28, 2010, 1:53 p.m. (June 28, 2010, 1:53 p.m.)

Certainly if you ended up whacking it anyway, you shouldn't keep it.  Is there anything useful in the queue that's not in this set of changes that you posted for review?

src/sim/Root.py (Diff revision 3)

I don't understand why you overload both __new__ and __init__.  Seems like everything should go in __new__. (i.e. why not just fatal() in __new__?)  I think you might also need to pass **kwargs to SimObject.__new__.

All this said, I'm not sure how __new__ interacts with a metaclass, so I could be wrong.

Steve Reinhardt June 28, 2010, 1:17 p.m. (June 28, 2010, 1:17 p.m.)

Hmm, I think you're right, this design was because I was initially going to print a warning instead of failing on a second instantiation, but then I gave up on that and didn't rethink the design.

The comment that's there discusses why I don't pass kwargs to SimObject.__new__... I agree it seems wrong, but that's the way it is.

Nathan Binkert June 28, 2010, 1:52 p.m. (June 28, 2010, 1:52 p.m.)

Ah, sorry about not reading the comment.  Upon thinking about it, I remember this problem, and I agree that it is pretty odd.  It is simply because object() takes no parameters, but it seems to me that it would be better if object just ignored all parameters.  Anyway...

Diff:

Revision 4 (+52 -19)

Show changes

	configs/common/Simulation.py
	configs/example/memtest.py
	configs/example/memtest-ruby.py
	configs/example/rubytest.py
	configs/splash2/cluster.py
	configs/splash2/run.py
	src/python/m5/SimObject.py
	src/python/m5/simulate.py
	2 more

Description:

		sim: make Python Root object a singleton
~		Enforce that the Python Root SimObject is instantiated only once.
~		The C++ Root object already panics if more than one is created.
~		This change avoids the need to track what the root object
~		is, since it's available from Root._getInstance() (if it exists).
~		It's now redundant to have the user pass the root object
~		to functions like instantiate() (though for backwards compatibility
~		we continue to support this for now with a warning).
	~	Enforce that the Python Root SimObject is instantiated only
	~	once. The C++ Root object already panics if more than one is
	~	created. This change avoids the need to track what the root
	~	object is, since it's available from Root.getInstance() (if it
	~	exists). It's now redundant to have the user pass the root
	~	object to functions like instantiate(), checkpoint(), and
	~	restoreCheckpoint(), so that arg is gone. Users who use
	+	configs/common/Simulate.py should not notice.

You have a pending review.

Review Board 2.0.15

This change has been marked as submitted.

Screenshots

Files

Diff:

Diff:

Diff:

Description: