nathan binkert wrote: >> Stepping back a bit, part of the complexity stems from the interface >> between the user's simulation script and the core code: when you call >> instantiate(), there's no indication whether or not you'll be >> restoring from a checkpoint, so we have to wait for the call to >> simulate() to do the startup() code, since that's the first point >> where we know that any checkpoint that's going to be loaded will be >> loaded. (Which requires a separate flag in the core code to detect >> the first time you call simulate()... not as pretty as it could be.) >> > I didn't realize that the whole problem was that the overwrite init() > with unserialize() was a problem. > > >> In fact there's no reason I can see to separate instantiate() and >> restoreCheckpoint(), and looking at configs/common/Simulate.py it >> shouldn't be hard to get rid of the restoreCheckpoint() call and just >> pass an optional checkpoint file to instantiate(). Ignoring the issue >> at hand, we could simplify the status quo above to: >> > Sounds fine to me. > > >> if checkpoint arg provided: >> for each SimObject: >> if checkpoint has section for this SimObject: >> call unserialize() >> else: >> call unserializeNoData() >> else: >> for each SimObject: >> call initWithoutCheckpoint() >> > > I don't have a major objection to the code, but what if we instead > added a flags argument to startup? > > checkpoint_data = set() > if checkpoint arg provided: > for obj in SimObject: > if checkpoint has section for obj: > checkpoint_data.add(obj) > obj.unserialize() > > for obj in SimObject: > obj.startup(checkpoint arg provided, checkpoint_data[obj]) > > I'm just slightly concerned about adding yet more functions to the > startup process that could be confusing. It also seems pretty likely > that a lot of the code in startup() won't care about the status of the > flags. In fact, you've only really demonstrated one place where it's > clearly an issue. The one issue that I see with this is that startup > isn't called at instantiate(), but rather when simulate() is called > the first time. If a user wanted to do something in python (which we > don't really support yet) after instantiate(), but before simulate(), > this could get confusing. I still kinda like unifying the > unserializeNoData() and initNoCheckpoint() as a single function with > flags. > > In python, there's __new__, __init__, and __setstate__. Which > functions are called actually depend on how things are pickled. There > __getinitargs__, __getnewargs__, and __getstate__. I think the > process is something like call __new__ with the arguments from > __getnewargs__ if the class defined one. Don't call __init__ unless > the class provided __getinitargs__ in which case, pass the args from > that function to __init__. Call __setstate__ with the arguments > returned from __getstate__. I'm not sure if we can get inspired by > this or not, but it is perhaps worth thinking about. I'll admit that > when I write code that uses pickle, I often wish that we had something > nicer in M5, then again, that probably has mostly to do with the nice > support for the actual serialization of the contents as opposed to the > overall machinery. > > >> The System/Process code that's at issue right now should be moved into >> initWithoutCheckpoint(), I think. I don't know that there's any need >> for the unserializeNoData() method, but now might be a good time to >> add it just in case. >> > > >> I don't mean to preclude Nate's idea of multi-phase init() and >> startup() calls, but I think that's somewhat independent of the issue >> I'm facing. >> > I see now. The multiphase init would work fine if you're happy having > unserialize overwrite some init() stuff. I'm not positive about init, > but I know that in the case of startup(), there are (or at least were) > some cases where some initialization code was done on the event queue > at tick 0 to ensure that it happened after all startup() calls were > done. If you like the change startup version of this, we should > probably add the multiphase thing at the same time. > > Nate > _______________________________________________ > m5-dev mailing list > [email protected] > http://m5sim.org/mailman/listinfo/m5-dev >
I was actually going to suggest passing a flag to startup rather than a new function. Gabe _______________________________________________ m5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/m5-dev
