Bug 190906 – Try ripping out inferred types because it might be a performance improvement

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Saam Barati]

...

[Saam Barati]

Doing this now

[Saam Barati]

Created attachment 353310 [details]
WIP

Test out EWS

[EWS Watchlist]

Attachment 353310 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/llint/LLIntOffsetsExtractor.cpp:59:  Alphabetical sorting problem.  [build/include_order] [4]
Total errors found: 1 in 18 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Saam Barati]

On Mac, this looks like:
JetStream 2: neutral - 0.8% improvement based on HW
Speedometer 2: neutral - 1% improvement based on HW

I still need to run on iOS

[Saam Barati]

(In reply to Saam Barati from comment #4)
> On Mac, this looks like:
> JetStream 2: neutral - 0.8% improvement based on HW
> Speedometer 2: neutral - 1% improvement based on HW
Make this: Speedometer 2: neutral - 1.75%
> 
> I still need to run on iOS

[Saam Barati]

Note: If we decide to keep this code, there is LLInt code that's wrong for BigInt in put_by_id

[Saam Barati]

Created attachment 359121 [details]
patch

[EWS Watchlist]

Attachment 359121 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/llint/LLIntOffsetsExtractor.cpp:59:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/JavaScriptCore/ChangeLog:7:  Line contains tab character.  [whitespace/tab] [5]
Total errors found: 2 in 20 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Saam Barati]

Created attachment 359187 [details]
patch

Try to fix 32-bit and non-mac platform build issues

[EWS Watchlist]

Attachment 359187 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/llint/LLIntOffsetsExtractor.cpp:59:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/JavaScriptCore/ChangeLog:7:  Line contains tab character.  [whitespace/tab] [5]
Total errors found: 2 in 21 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Yusuke Suzuki]

Comment on attachment 359187 [details]
patch

View in context: https://bugs.webkit.org/attachment.cgi?id=359187&action=review

r=me with additional cleanup suggestions.

> Source/JavaScriptCore/bytecode/PutByIdFlags.h:41
>  inline PutByIdFlags encodeStructureID(StructureID id)

This function is no longer used.

> Source/JavaScriptCore/bytecode/PutByIdFlags.h:54
>  #if USE(JSVALUE64)

Ditto.

> Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h:-2921
> -                    result.merge(
> -                        m_graph.inferredValueForProperty(
> -                            value, uid, status[i].offset(), m_state.structureClobberState()));

This would perform tryGetConstantProperty, don't we need to do this?

> Source/JavaScriptCore/llint/LowLevelInterpreter64.asm:-1354
> -    loadi OpPutById::Metadata::flags[t5], t1

Now, I think that this metadata.flag is no longer necessary. All the information can be fit into byte code.flag.

[Saam Barati]

Comment on attachment 359187 [details]
patch

View in context: https://bugs.webkit.org/attachment.cgi?id=359187&action=review

>> Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h:-2921
>> -                            value, uid, status[i].offset(), m_state.structureClobberState()));
> 
> This would perform tryGetConstantProperty, don't we need to do this?

Yes. I shouldn't have removed it.

[Saam Barati]

Created attachment 359196 [details]
patch for landing

[EWS Watchlist]

Attachment 359196 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/llint/LLIntOffsetsExtractor.cpp:59:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/JavaScriptCore/ChangeLog:7:  Line contains tab character.  [whitespace/tab] [5]
Total errors found: 2 in 22 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[WebKit Commit Bot]

Comment on attachment 359196 [details]
patch for landing

Rejecting attachment 359196 [details] from commit-queue.

Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-01', 'land-attachment', '--force-clean', '--non-interactive', '--parent-command=commit-queue', 359196, '--port=mac']" exit_code: 2 cwd: /Volumes/Data/EWS/WebKit

Last 5000 characters of output:
u must commit a file with tabs, use svn propset to set the "allow-tabs" property.
W: 5b0710c656a3667176a0e697ddf2b0be91a76beb and refs/remotes/origin/master differ, using rebase:
:040000 040000 9902d06c4513992a85e4ce8903cea235fec10a61 26fd48e8dfc4d9f56967ce8420b34ecca68dfba3 M	Source
Current branch master is up to date.
ERROR: Not all changes have been committed into SVN, however the committed
ones (if any) seem to be successfully integrated into the working tree.
Please see the above messages for details.


Failed to run "['git', 'svn', 'dcommit', '--rmdir']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit

Committing to http://svn.webkit.org/repository/webkit/trunk ...
	D	Source/JavaScriptCore/dfg/DFGDesiredInferredType.h
	D	Source/JavaScriptCore/dfg/DFGInferredTypeCheck.cpp
	D	Source/JavaScriptCore/dfg/DFGInferredTypeCheck.h
	D	Source/JavaScriptCore/runtime/InferredStructure.cpp
	D	Source/JavaScriptCore/runtime/InferredStructure.h
	D	Source/JavaScriptCore/runtime/InferredStructureWatchpoint.cpp
	D	Source/JavaScriptCore/runtime/InferredStructureWatchpoint.h
	D	Source/JavaScriptCore/runtime/InferredType.cpp
	D	Source/JavaScriptCore/runtime/InferredType.h
	D	Source/JavaScriptCore/runtime/InferredTypeInlines.h
	D	Source/JavaScriptCore/runtime/InferredTypeTable.cpp
	D	Source/JavaScriptCore/runtime/InferredTypeTable.h
	M	Source/JavaScriptCore/CMakeLists.txt
	M	Source/JavaScriptCore/ChangeLog
	M	Source/JavaScriptCore/JavaScriptCore.xcodeproj/project.pbxproj
	M	Source/JavaScriptCore/Sources.txt
	M	Source/JavaScriptCore/UnifiedSources-input.xcfilelist
	M	Source/JavaScriptCore/bytecode/AccessCase.cpp
	M	Source/JavaScriptCore/bytecode/BytecodeList.rb
	M	Source/JavaScriptCore/bytecode/Fits.h
	M	Source/JavaScriptCore/bytecode/PutByIdFlags.cpp
	M	Source/JavaScriptCore/bytecode/PutByIdFlags.h
	M	Source/JavaScriptCore/bytecode/PutByIdStatus.cpp
	M	Source/JavaScriptCore/bytecode/PutByIdVariant.cpp
	M	Source/JavaScriptCore/bytecode/PutByIdVariant.h
	M	Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h
	M	Source/JavaScriptCore/dfg/DFGAbstractValue.cpp
	M	Source/JavaScriptCore/dfg/DFGAbstractValue.h
	M	Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp
	M	Source/JavaScriptCore/dfg/DFGConstantFoldingPhase.cpp
	M	Source/JavaScriptCore/dfg/DFGDesiredWatchpoints.cpp
	M	Source/JavaScriptCore/dfg/DFGDesiredWatchpoints.h
	M	Source/JavaScriptCore/dfg/DFGFixupPhase.cpp
	M	Source/JavaScriptCore/dfg/DFGGraph.cpp
	M	Source/JavaScriptCore/dfg/DFGGraph.h
	M	Source/JavaScriptCore/dfg/DFGNode.h
	M	Source/JavaScriptCore/dfg/DFGObjectAllocationSinkingPhase.cpp
	M	Source/JavaScriptCore/dfg/DFGSafeToExecute.h
	M	Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp
	M	Source/JavaScriptCore/generator/DSL.rb
	M	Source/JavaScriptCore/heap/Heap.cpp
	M	Source/JavaScriptCore/jit/AssemblyHelpers.cpp
	M	Source/JavaScriptCore/jit/AssemblyHelpers.h
	M	Source/JavaScriptCore/jit/JITPropertyAccess.cpp
	M	Source/JavaScriptCore/jit/JITPropertyAccess32_64.cpp
	M	Source/JavaScriptCore/jit/Repatch.cpp
	M	Source/JavaScriptCore/llint/LLIntOffsetsExtractor.cpp
	M	Source/JavaScriptCore/llint/LLIntSlowPaths.cpp
	M	Source/JavaScriptCore/llint/LowLevelInterpreter.asm
	M	Source/JavaScriptCore/llint/LowLevelInterpreter32_64.asm
	M	Source/JavaScriptCore/llint/LowLevelInterpreter64.asm
	M	Source/JavaScriptCore/runtime/JSModuleLoader.h
	M	Source/JavaScriptCore/runtime/JSObjectInlines.h
	M	Source/JavaScriptCore/runtime/Structure.cpp
	M	Source/JavaScriptCore/runtime/Structure.h
	M	Source/JavaScriptCore/runtime/StructureInlines.h
	M	Source/JavaScriptCore/runtime/VM.cpp
	M	Source/JavaScriptCore/runtime/VM.h

ERROR from SVN:
A repository hook failed: Commit blocked by pre-commit hook (exit code 1) with output:

    The following files contain tab characters:

        trunk/Source/JavaScriptCore/ChangeLog

    Please use spaces instead to indent.
    If you must commit a file with tabs, use svn propset to set the "allow-tabs" property.
W: 5b0710c656a3667176a0e697ddf2b0be91a76beb and refs/remotes/origin/master differ, using rebase:
:040000 040000 9902d06c4513992a85e4ce8903cea235fec10a61 26fd48e8dfc4d9f56967ce8420b34ecca68dfba3 M	Source
Current branch master is up to date.
ERROR: Not all changes have been committed into SVN, however the committed
ones (if any) seem to be successfully integrated into the working tree.
Please see the above messages for details.


Failed to run "['git', 'svn', 'dcommit', '--rmdir']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit
Updating OpenSource
From https://git.webkit.org/git/WebKit
   4320f2cdf04..a364fa76fb3  master     -> origin/master
Partial-rebuilding .git/svn/refs/remotes/origin/master/.rev_map.268f45cc-cd09-0410-ab3c-d52691b4dbfc ...
Currently at 240016 = 4320f2cdf04c35333326d85e2625b21ec6803bdf
r240017 = a364fa76fb35b7cc445c092b976a5d2e121a43d0
Done rebuilding .git/svn/refs/remotes/origin/master/.rev_map.268f45cc-cd09-0410-ab3c-d52691b4dbfc
First, rewinding head to replay your work on top of it...
Fast-forwarded master to refs/remotes/origin/master.

Full output: https://webkit-queues.webkit.org/results/10765054

[Saam Barati]

Created attachment 359228 [details]
patch for landing

[WebKit Commit Bot]

Comment on attachment 359228 [details]
patch for landing

Clearing flags on attachment: 359228

Committed r240023: <https://trac.webkit.org/changeset/240023>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/47303601>

[Saam Barati]

Results are:
1% faster Speedometer 2
1% faster ARES6
0-1% faster JetStream2
1% slower JetStream