- 
                Notifications
    You must be signed in to change notification settings 
- Fork 1.8k
[GR-36894] Add DynamicObject nodes. #12238
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
          
     Open
      
      
            graalvmbot
  wants to merge
  14
  commits into
  master
  
    
      
        
          
  
    
      Choose a base branch
      
     
    
      
        
      
      
        
          
          
        
        
          
            
              
              
              
  
           
        
        
          
            
              
              
           
        
       
     
  
        
          
            
          
            
          
        
       
    
      
from
bd/GR-36894-dynamicobjectnodes
  
      
      
   
  
    
  
  
  
 
  
      
    base: master
Could not load branches
            
              
  
    Branch not found: {{ refName }}
  
            
                
      Loading
              
            Could not load tags
            
            
              Nothing to show
            
              
  
            
                
      Loading
              
            Are you sure you want to change the base?
            Some commits from the old base branch may be removed from the timeline,
            and old review comments may become outdated.
          
          
                
     Open
            
            
          
      
        
          +5,595
        
        
          −1,239
        
        
          
        
      
    
  
Conversation
  
    
      This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
      Learn more about bidirectional Unicode characters
    
  
  
    
    69cd581    to
    47974e6      
    Compare
  
    7c6babc    to
    d51a949      
    Compare
  
    * Faster in interpreter and better footprint than DynamicObjectLibrary,
  because Truffle libraries do not support host inlining and have footprint overhead.
* Use a more optimized guard for set() with 2 Shapes.
* Only inline cache 1 Shape for GetShapeFlagsNode.
  * An extra read seems better than more control flow.
* Migrate SL to DynamicObject nodes.
* Check the old Shape Assumption in the @Specialization.assumptions
  * This avoids an extra call to putGeneric() for host inlining,
    and ensures the specialization gets removed if the oldShape becomes invalid.
* Use putFlags instead of cachedPutFlags to fold during host inlining.
  * This saves around 6 node cost for DynamicObject.PutFixedKeyNode.
  * It is only observable if multiple PutNode#put*() methods are used (= different put flags) in the native image.
* Remove calls to Location#isFinal(), it's always false.
* Link the replacement nodes in DynamicObjectLibrary.
* Remove {PutNode,PutFixedKeyNode}#putInt, it boxes anyway so there is no value
  * Also other variants like putDouble are missing.
* Use @fallback to avoid potential issues with a Shape becoming invalid concurrently.
* Remove KeyEqualsNode and just compare keys by identity
  * Reduces subTreeCost of SLReadPropertyNode.readSLObject from 307 to 245.
* Use the same Shape check both in guards and specialization body.
* Merge inline caches of PutNode and InlinedConstKey into one.
* Merge inline caches of {PutNode,PutFixedKeyNode} and PutCache into one.
* Handle invalid shapes without removing all cached specializations
  * Removing the specialization instance itself is fine, the problem is
    removing all cache specialization due to the `replaces` on doUncached.
* Add a property read micro benchmark for SL
* Port documentation from DynamicObjectLibrary to DynamicObject nodes
* Update docs, 16 bit are actually allowed for user Shape flags
* Migrate DynamicObjectBenchmark and DynamicObjectPartialEvaluationTest
* [EXPERIMENT] Manually inline ExtLocations.ObjectArrayLocation in GetNode
* [EXPERIMENT] Manually inline ExtLocations.IntArrayLocation in GetNode
* [EXPERIMENT] Manually inline ExtLocations.ObjectArrayLocation in PutNode
* Cache the Location instead of Property in DynamicObject nodes to avoid extra indirection in interpreter
* Add interpreter benchmarks for DynamicObject.{GetNode,PutNode}
* It is enough to check the new Shape Assumption in PutNode
  * If the old Shape Assumption is invalidated, it will also invalidate the new Shape Assumption.
  * Fix bug where doCached was used with an old obsolete shape
* [GR-69326] Avoid duplicate property lookup in DynamicObject.PutNode
* Extract reusePropertyLookup() and fix RemoveKeyNode
* Run all DynamicObject tests with the new DynamicObject nodes too.
* Ignore spotbugs warnings in generated code.
Co-authored-by: Andreas Woess <[email protected]>
    Move index, field info, and final assumption to Location base class. Merge array and field location classes. Add non-virtual Location.canStoreValue(). Add non-virtual Location.get and set methods. Make isAssumedFinal() and isConstant() non-virtual. Add exact-type fast path for type.isInstance(value).
Ensure extension arrays are seen as non-null in host compilation. Remove DynamicField annotation from the array fields to avoid marking them as unsafe-accessed.
d51a949    to
    ea83644      
    Compare
  
    
  
    Sign up for free
    to join this conversation on GitHub.
    Already have an account?
    Sign in to comment
  
      
  Add this suggestion to a batch that can be applied as a single commit.
  This suggestion is invalid because no changes were made to the code.
  Suggestions cannot be applied while the pull request is closed.
  Suggestions cannot be applied while viewing a subset of changes.
  Only one suggestion per line can be applied in a batch.
  Add this suggestion to a batch that can be applied as a single commit.
  Applying suggestions on deleted lines is not supported.
  You must change the existing code in this line in order to create a valid suggestion.
  Outdated suggestions cannot be applied.
  This suggestion has been applied or marked resolved.
  Suggestions cannot be applied from pending reviews.
  Suggestions cannot be applied on multi-line comments.
  Suggestions cannot be applied while the pull request is queued to merge.
  Suggestion cannot be applied right now. Please check back later.
  
    
  
    
tl;dr: Introduce a new
Node-based API for dealing withDynamicObject, as a more lightweight replacement forDynamicObjectLibrary.