s5z / zsim

A fast and scalable x86-64 multicore simulator
GNU General Public License v2.0
326 stars 182 forks source link

ZSim OOO timing model improvements #213

Open dzhang50 opened 6 years ago

dzhang50 commented 6 years ago

I've been using ZSim for my PhD thesis work on accelerating parallel graph analytics. I have since graduated and gone to industry. I recently received an email from a professor inquiring about the accuracy improvements I made to ZSim, briefly mentioned in Section 6.2 in my paper: https://dl.acm.org/citation.cfm?id=3173197

Through our discussions, I agreed to provide some of these changes to the public for the benefit of the academic community. These changes are predominantly updates to the OOO timing model, but also include things like an MD1 network model to replace the fixed-latency network model. Some of these changes may significantly alter ZSim reported performance on some benchmarks - in theory for the better.

I'm wondering how you think I should proceed. Should I send a pull request for each of these changes, or do you think it would be better for me to fork off a different variant of ZSim (i.e. create a ZSim++)? I've started things off by issuing a very simple pull request consisting of adding a more detailed config file: https://github.com/s5z/zsim/pull/212

gaomy3832 commented 6 years ago

Eventually it is up to Daniel or someone from the MIT group to provide the final answer, but I would like to share some of my thoughts as I am pretty interested in the improvements you made :)

In my opinion, if the code change is relatively local, within a few files (e.g., I would assume the OOO improvements are mostly in ooo_core.h/cpp and ooo_core_recorder.h/cpp), and does not change the interface of any module, then it should and can be easily merged into the main repo. A pull request will be great.

If the change is kind of global, touching many files and altering the interface, then it will be hard to merge due to conflicts with others. For example, the new network model may change the interface of cache and memory (or not, which is great). In this case, a pull request may make a too heavy update to the main repo and cause issues to someone else.

Of course, the improvements should be general, which is the case for your thing. The zsim-NVM project is specific to NVM, so they maintain a separate repo.

And finally, try to be consistent in code style :)

dzhang50 commented 5 years ago

Since Daniel and the other authors never responded to my email or PR, I decided to create my own fork of ZSim called ZSim++:

https://github.com/dzhang50/zsim-plusplus

It's a separate repository rather than an official Github fork for the reasons documented here:

https://www.niels-ole.com/ownership/2018/03/16/github-forks.html https://github.com/yigit/android-priority-jobqueue/issues/58

I have been (very slowly) cleaning up my research code and committing one feature at a time to my new repo. So far, I have added the simple config file, DDR4 support, floating-point stats (useful for things like IPC), and the simple MD1 network model. There's still plenty of changes to come, including a ton of changes to the OOO core model (including serious bug fixes). I hope that other people who have been using ZSim for a long time (such as @gaomy3832 ) will also consider contributing their own code to ZSim++, provided the changes are sufficiently general.

Dan