Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ZSim OOO timing model improvements #213

Open
dzhang50 opened this issue Jul 16, 2018 · 2 comments
Open

ZSim OOO timing model improvements #213

dzhang50 opened this issue Jul 16, 2018 · 2 comments

Comments

@dzhang50
Copy link

dzhang50 commented Jul 16, 2018

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: #212

@gaomy3832
Copy link
Contributor

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
Copy link
Author

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
yigit/android-priority-jobqueue#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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants