Time: Add a mechanism to prevent M5 from running faster than real time.

Information
Submitter:	Gabe Black
Repository:	gem5
Branch:
Bugs:
Depends On:
Reviewers
Groups:	Default
People:	ali, gblack, nate, stever

Description

Time: Add a mechanism to prevent M5 from running faster than real time.

M5 skips over any simulated time where it doesn't have any work to do. When
the simulation is active, the time skipped is short and the work done at any
point in time is relatively substantial. If the time between events is long
and/or the work to do at each event is small, it's possible for simulated time
to pass faster than real time. When running a benchmark that can be good
because it means the simulation will finish sooner in real time. When
interacting with the real world through, for instance, a serial terminal or
bridge to a real network, this can be a problem. Human or network response time
could be greatly exagerated from the perspective of the simulation and make
simulated events happen "too soon" from an external perspective.

This change adds the capability to force the simulation to run no faster than
real time. It does so by scheduling a periodic event that checks to see if
its simulated period is shorter than its real period. If it is, it stalls the
simulation until they're equal. This is called time syncing.

A future change could add pseudo instructions which turn time syncing on and
off from within the simulation. That would allow time syncing to be used for
the interactive parts of a session but then turned off when running a
benchmark using the m5 utility program inside a script. Time syncing would
probably not happen anyway while running a benchmark because there would be
plenty of work for M5 to do, but the event overhead could be avoided.

Testing Done

SConstruct (Diff revision 1)

What happens if this fails?

Steve Reinhardt Jan. 10, 2011, 5:12 a.m. (Jan. 10, 2011, 5:12 a.m.)

I'm fine with just not having the sync functionality if librt is unavailable, if there are platforms that don't support it, but it does seem like we need some more code to handle that case.

src/sim/root.hh (Diff revision 1)

I think that it'd be better if we just put the declaration of the Root SimObject here.  The reason it wasn't was probably just because nobody ever needed it before.

This along with some sort of function like "Root *root();" to get the actual root object would eliminate the need for all of these wrapper functions.  (Which don't buy us anything as far as I can tell).

Steve Reinhardt Jan. 10, 2011, 5:13 a.m. (Jan. 10, 2011, 5:13 a.m.)
```
I agree.
```

src/sim/root.hh (Diff revision 1)

brace on the same line as namespace.  Steve just went through and fixed a zillion instances of the namespace.  It should probably be in the style guide.

src/sim/root.hh (Diff revision 1)

should be:
} // namespace TimeSync

src/sim/root.cc (Diff revision 1)

Perhaps you can use base/time.hh?  (and update it to use timespec

If not, this should be <ctime>

src/sim/root.cc (Diff revision 1)

Probably make this private (and _root).

Then make a static accessor.  I say this because we may have to add a lock around it in the future parallel version.

src/sim/root.cc (Diff revision 1)

Ok, I apologize for being overly anal, but local variables are not camelCase, but with_underscores.

src/sim/root.cc (Diff revision 1)

Here's an example of where I'd like a temporary var (no requirement or anything like that).  I think this would mean the same thing:

int64_t remainder = period() - nsDiff;
if (remainder > 0 && remainder > _spinThreshold)

src/sim/root.cc (Diff revision 1)

you could use remainder here.

src/sim/root.cc (Diff revision 1)

If librt isn't available, will this work?  Is there an alternative?

SConstruct (Diff revision 1)

clock_nanosleep isn't particularly portable. What about usleep() or nanosleep()? Both are in libc as opposed to librt and seem to exist on both my linux and and os x machine.

src/sim/root.cc (Diff revision 1)

How about just gettimeofday()? it also only support usec resolution, but it's very portable.

src/sim/root.cc (Diff revision 1)

Thanks for taking the time to do this!

Steve Reinhardt Jan. 10, 2011, 7:43 a.m. (Jan. 10, 2011, 7:43 a.m.)

Ditto... I meant to say something in my review but forgot... despite the nitpicking, thanks for whipping up a solution so quickly.  Otherwise we'd have nothing to nitpick :-)

Diff:

Revision 2 (+161 -7)

Show changes

	configs/example/fs.py
	src/sim/Root.py
	src/sim/SConscript
	src/sim/root.hh
	src/sim/root.cc

src/sim/root.hh (Diff revision 2)

@file

Gabe Black Jan. 18, 2011, 8:44 a.m. (Jan. 18, 2011, 8:44 a.m.)
```
Done
```

src/sim/root.hh (Diff revision 2)

Doxygen comments are needed throughout

Gabe Black Jan. 18, 2011, 8:41 a.m. (Jan. 18, 2011, 8:41 a.m.)
```
Done
```

src/sim/root.hh (Diff revision 2)

I would like to see a detailed comment about how it works here. e.g. the second paragraph from your commit message.

Gabe Black Jan. 18, 2011, 8:34 a.m. (Jan. 18, 2011, 8:34 a.m.)
```
Done
```

src/sim/root.hh (Diff revision 2)

This seems large enough that it should just go in the cc file

Gabe Black Jan. 18, 2011, 8:34 a.m. (Jan. 18, 2011, 8:34 a.m.)
```
Done
```

src/sim/root.cc (Diff revision 2)

What sleep is this calling?

Gabe Black Jan. 18, 2011, 8:34 a.m. (Jan. 18, 2011, 8:34 a.m.)
```
The one defined in time.hh for M5's custom Time class.
```

Diff:

Revision 3 (+194 -7)

Show changes

	configs/example/fs.py
	src/sim/Root.py
	src/sim/SConscript
	src/sim/root.hh
	src/sim/root.cc

Looks good to me.

src/sim/root.hh (Diff revision 3)

No big deal, but we usually have a comment here

Gabe Black Jan. 18, 2011, 3:01 p.m. (Jan. 18, 2011, 3:01 p.m.)
```
Done
```

src/sim/root.cc (Diff revision 3)

Seems like we should have some sort of mechanism for doing this.

Gabe Black Jan. 18, 2011, 3:12 p.m. (Jan. 18, 2011, 3:12 p.m.)

If you add a function for it I'll use it. We should be careful, though, since when nanoseconds (or microseconds or milliseconds) spill into seconds might get a little tricky. Actually, right now nanoseconds, microseconds, and milliseconds all change the same data, but seconds are separate. That might not be obvious to somebody not in the know.

src/sim/root.hh (Diff revision 3)

Now that these methods are part of Root, I think they need a little more context in their names, e.g.:
timeSyncEnabled()
timeSyncPeriod()
timeSyncEnable()
That makes them more parallel with the Python param names too.

Gabe Black Jan. 18, 2011, 3:12 p.m. (Jan. 18, 2011, 3:12 p.m.)
```
Done
```

src/sim/root.hh (Diff revision 3)

I think this function and the next one should be in the .cc file and not in the .hh... they're not completely trivial and they're not performance-critical.

Gabe Black Jan. 18, 2011, 3:12 p.m. (Jan. 18, 2011, 3:12 p.m.)
```
Done
```

Diff:

Revision 4 (+198 -7)

Show changes

	configs/example/fs.py
	src/sim/Root.py
	src/sim/SConscript
	src/sim/root.hh
	src/sim/root.cc

Looks great!

You have a pending review.

Review Board 2.0.15

This change has been marked as submitted.

Screenshots

Files

Diff:

Diff:

Diff: