Review Board 2.0.15


sim: clean up child handling

Review Request #77 - Created July 29, 2010 and submitted

Information
Steve Reinhardt
gem5
Reviewers
Default 
sim: clean up child handling
The old code for handling SimObject children was kind of messy,
with children stored both in _values and _children, and
inconsistent and potentially buggy handling of SimObject
vectors.  Now children are always stored in _children, and
SimObject vectors are consistently handled using the
SimObjectVector class.

Also, by deferring the parenting of SimObject-valued parameters
until the end (instead of doing it at assignment), we eliminate
the hole where one could assign a vector of SimObjects to a
parameter then append to that vector, with the appended objects
never getting parented properly.

This patch induces small stats changes in tests with data races
due to changes in the object creation & initialization order.
The new code does object vectors in order and so should be more
stable.

   

  


 


 

  

  









  
 


  

 
















 



 

  


 


  



   

   
Nathan Binkert

   
Posted  (Aug. 6, 2010, 1:57 a.m.)


  


  


  

   
At some point, you and I should really go through and look at what we have here and see if we actually understand what is going on.  Nothing in this diff looked wrong, so I think it's fine to go, but there are so many things that are questionable.  At the very least it would be nice if we could work together for an hour or so and just comment the code.  It's certainly crazy complicated.



  

   

   
    
    
   


  




   







    

     
     

      

       
        

         
          src/python/m5/SimObject.py
          

           (Diff revision 1)

          
         
        

       
       
        
 


        
 


       
      

     

    

    


     
Do we explicitly do anything to prevent the same name from going into both _values and _children?




  

   

   
    
    

   
  1. 
    
    
     
    
      
     
    
     
    No, in fact if you have a SimObject-valued parameter that's also a child then it will be in both.  I think that's one of the key changes here, that all children are found in _children, even if they're also in _values.
    
    
    
   
    2. 

   


  


    


    

     
     

      

       
        

         
          src/python/m5/SimObject.py
          

           (Diff revision 1)

          
         
        

       
       
        
 


        
 


       
      

     

    

    


     
So, does this mean that if I instantiate a SimObject called foo (e.g. bar = foo()), I won't get updates to children made directly to foo?  If so, is that what we want?  It's not exactly consistent with the values.




  

   

   
    
    

   
  1. 
    
    
     
    
      
     
    
     
    It's not a semantic change... note the code immediately above with the comment "clone SimObject-valued parameters", which was there all along.  The only thing that's different is that the _children dict is getting initialized explicitly up front instead of implicitly later (don't ask where it used to happen, but note that it used to be empty at this point).

Reassigning a child won't get inherited, but since the child objects themselves are getting cloned, changes to parameter values in the children of foo (which are the ancestors of the children of bar) will get reflected in the children of bar.  Again, I think that's the way it's always been.
    
    
    
   
    2. 

   


  


    


    

     
     

      

       
        

         
          src/python/m5/SimObject.py
          

           (Diff revision 1)

          
         
        

       
       
        
 


        
 


       
      

     

    

    


     
I just noticed that we have the same __getattr__ in both SimObject and MetaSimObject.  That seems a little bit odd.  Do we even need the latter?




  

   

   
    
    

   
  1. 
    
    
     
    
      
     
    
     
    They're not quite the same, as the SimObject version has the redirection to self._ccObject.  Don't we need the MetaSimObject version to support querying parameter values on classes?
    
    
    
   
    2. 

   


  


    


    

     
     

      

       
        

         
          src/python/m5/SimObject.py
          

           (Diff revision 1)

          
         
        

       
       
        
 


        
 


       
      

     

    

    


     
Shouldn't these functions start with an underscore?  Do we expect them to be used by users?




  

   

   
    
    

   
  1. 
    
    
     
    
      
     
    
     
    It's unlikely that a user will want to call these directly, but it's not really a problem if they do.
    
    
    
   
    2.