Closed Bug 511567 Opened 15 years ago Closed 15 years ago

atomize unit strings at birth, and bypass lock when atomizing strings of length 1

Categories

(Core :: JavaScript Engine, defect)

Product:
Core
Component:
JavaScript Engine
Platform:
x86
macOS
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
VERIFIED FIXED
Tracking Flags:
Tracking Status
status1.9.2 --- beta1-fixed

People

(Reporter: gal, Assigned: gal)

References

Details

(Keywords: verified1.9.2, Whiteboard: fixed-in-tracemonkey)

Attachments

(1 file, 2 obsolete files)

patch
1.94 KB, patch
shaver
: review+
Details | Diff | Splinter Review 
About 50% of strings in SS are unit strings (50k). Bypassing the lock for them should be a nice win.
Assignee: general → gal
Attached patch patch (obsolete) — — Splinter Review 

    
    

    
  
About a 5ms win for a MT shell.

unpack-code:  1.044x as fast     98.6ms +/- 0.2%    94.5ms +/- 0.2%  significant

    
  

        Attachment #395481 -
        Flags: review?(shaver)

    
    

    
  

      
      
      
      

        Attached patch
          
          
        patch (obsolete)
        — 
          — Splinter Review
      

    


  

        Attachment #395481 -
        Attachment is obsolete: true

        Attachment #395489 -
        Flags: review?(shaver)

        Attachment #395481 -
        Flags: review?(shaver)

    
    

    
  

      
      
      
      

        Attached patch
          
          
        patch
        — 
          — Splinter Review
      

    


  

        Attachment #395489 -
        Attachment is obsolete: true

        Attachment #395492 -
        Flags: review?(shaver)

        Attachment #395489 -
        Flags: review?(shaver)

    
    

    
  
Comment on attachment 395492 [details] [diff] [review]
patch

Cool, thanks for the test. r=shaver

        Attachment #395492 -
        Flags: review?(shaver) → review+

    
    

    
  
http://hg.mozilla.org/tracemonkey/rev/3d98dade7db3
Whiteboard: fixed-in-tracemonkey

    
    

    
  
Good fix.

Style-wise, please use (JSAtom *) prevailing style (you agreed to do this, IIRC), and four-space indentation in the test.

/be

    
    

    
  
http://hg.mozilla.org/mozilla-central/rev/3d98dade7db3
Status: NEW → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED

    
  
Depends on: 512996

    
    

    
  
js/src/trace-test/tests/basic/testAtomize.js
v 1.9.3, 1.9.2
Status: RESOLVED → VERIFIED
Flags: in-testsuite+
Keywords: verified1.9.2

          You need to log in
          before you can comment on or make changes to this bug.
        






  

    
  







  

    

      
Attachment

      

      
      
      
      
    

    

      

        

          

            
General

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: