> On Sept. 11, 2015, 9:35 p.m., Brandon Potter wrote: > > Hi Nilay, I plan on posting a patch which enables multi-instance Ruby; it > > will come with a configuration file that can be used to test the > > functionality. So far, the patch works well enough with multi-instance > > that I have not noticed problems. > > > > However, this specific issue with the two static variables in this patch > > still exists; we must have unintentionally missed it. I do however think > > that it can be obviated by using the main method that we used in our patch. > > (Essentially, everything that was static before has been moved into the > > per-instance RubySystem object or obviated in some other way.) > > > > I'm am currently rebasing my patches from an older revision to the tip of > > the tree and encountering some problems with more recent Ruby patches. As > > soon as those are worked out, I will post my patch and we can discuss the > > merits/demerits. As a forewarning, the solution is not elegant but it does > > work. I'm sure that we can refactor it into something cleaner if necessary. > > Nilay Vaish wrote: > Brandon, I am aware of the approach that AMD is taking and I am opposed > to it. > Dereferencing global pointer on every single memory access is certainly > not > good for performance. For statistics, we should add an option to python > config file so that we process the stats file and combine everything. I > don't > see why combining the stats across the controllers should be the > simulator's job. > > Brandon Potter wrote: > Fortunately, since we'll reference it so often, we'll keep it warm in our > cache. > > In reality, I don't think it's as bad as you're suggesting; Ruby already > goes through machinations just to operate and the extra indirect references > __probably__ won't harm performance much. (If anything, the most reasonable > thing to complain about maintaining the changes to the source.) > > This is a functional problem that Ruby has had since it was integrated > into m5 and possibly before when it was still GEMS. In my opinion, that > trumps a %x percent performance slowdown. The limitation of only being able > to invoke a single Ruby instance is bonkers. > > On the upside, when this is fixed, no one will be able to argue that they > can't use Ruby to do multi-instance simulations and that they have to revert > to using the classic cache model!
> Fortunately, since we'll reference it so often, we'll keep it warm in our > cache. Given the amount of data cpu cores and ruby itself touches (with multiple systems it would be even more), I do not see why those variables would be in the cache. > In reality, I don't think it's as bad as you're suggesting; Ruby already > goes through machinations just to operate and the extra indirect references > probably won't harm performance much. (If anything, the most reasonable thing > to complain about maintaining the changes to the source.) And I would like those machinations to go away. I have in past complained about using the ruby_system pointer way, and I am going to stick to that. > This is a functional problem that Ruby has had since it was integrated > into m5 and possibly before when it was still GEMS. In my opinion, that > trumps a %x percent performance slowdown. The limitation of only being able > to invoke a single Ruby instance is bonkers. > On the upside, when this is fixed, no one will be able to argue that they > can't use Ruby to do multi-instance simulations and that they have to revert > to using the classic cache model! I am not opposed to having multiple ruby systems. This patch has been posted to do enable multiple systems. But having mutiple systems does not mean that we have to create multiple copies of static variables. One of the reasons people do not use Ruby is because of its performance. And doing any amount of more work, especially work that is not required at all, would further hit performance. Since you have missed out on these variables, I suggest that you do not take any further step. I'll soon post a solution which would call a function each time stats are dumped and combine stats that this patch converts from global to local. - Nilay ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3013/#review7161 ----------------------------------------------------------- On Aug. 9, 2015, 6:26 p.m., Nilay Vaish wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/3013/ > ----------------------------------------------------------- > > (Updated Aug. 9, 2015, 6:26 p.m.) > > > Review request for Default. > > > Repository: gem5 > > > Description > ------- > > Changeset 11050:8f8fc7ddc278 > --------------------------- > ruby: convert global statistical variables to local ones. > Several statistical quantities collected by ruby where recorded / published > using global variables. Since we want to have multiple ruby systems working > together, this is a hindrance. Hence, we are convert such global variables to > local ones. > > > Diffs > ----- > > src/mem/ruby/slicc_interface/AbstractController.hh 863d314f6356 > src/mem/ruby/slicc_interface/AbstractController.cc 863d314f6356 > src/mem/slicc/symbols/StateMachine.py 863d314f6356 > > Diff: http://reviews.gem5.org/r/3013/diff/ > > > Testing > ------- > > > Thanks, > > Nilay Vaish > > _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
