Bug 190906

Summary: Try ripping out inferred types because it might be a performance improvement
Product: WebKit Reporter: Saam Barati <saam>
Component: JavaScriptCoreAssignee: Saam Barati <saam>
Status: RESOLVED FIXED    
Severity: Normal CC: benjamin, commit-queue, ews-watchlist, fpizlo, ggaren, gskachkov, keith_miller, mark.lam, msaboff, rmorisset, ticaiolima, tzagallo, webkit-bug-importer, ysuzuki
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=186725
Attachments:
Description Flags
WIP
none
patch
none
patch
ysuzuki: review+
patch for landing
commit-queue: commit-queue-
patch for landing none

Description Saam Barati 2018-10-25 10:25:16 PDT
...
Comment 1 Saam Barati 2018-10-29 12:00:21 PDT
Doing this now
Comment 2 Saam Barati 2018-10-29 12:47:36 PDT
Created attachment 353310 [details]
WIP

Test out EWS
Comment 3 EWS Watchlist 2018-10-29 12:50:36 PDT
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.
Comment 4 Saam Barati 2018-10-31 16:18:13 PDT
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
Comment 5 Saam Barati 2018-10-31 16:27:38 PDT
(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
Comment 6 Saam Barati 2019-01-14 18:35:18 PST
Note: If we decide to keep this code, there is LLInt code that's wrong for BigInt in put_by_id
Comment 7 Saam Barati 2019-01-14 19:55:08 PST
Created attachment 359121 [details]
patch
Comment 8 EWS Watchlist 2019-01-14 19:57:00 PST
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.
Comment 9 Saam Barati 2019-01-15 11:53:21 PST
Created attachment 359187 [details]
patch

Try to fix 32-bit and non-mac platform build issues
Comment 10 EWS Watchlist 2019-01-15 11:56:35 PST
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.
Comment 11 Yusuke Suzuki 2019-01-15 12:32:50 PST
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.
Comment 12 Saam Barati 2019-01-15 12:35:42 PST
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.
Comment 13 Saam Barati 2019-01-15 13:08:44 PST
Created attachment 359196 [details]
patch for landing
Comment 14 EWS Watchlist 2019-01-15 13:12:27 PST
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.
Comment 15 WebKit Commit Bot 2019-01-15 16:12:54 PST
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
Comment 16 Saam Barati 2019-01-15 16:59:03 PST
Created attachment 359228 [details]
patch for landing
Comment 17 WebKit Commit Bot 2019-01-15 17:41:49 PST
Comment on attachment 359228 [details]
patch for landing

Clearing flags on attachment: 359228

Committed r240023: <https://trac.webkit.org/changeset/240023>
Comment 18 WebKit Commit Bot 2019-01-15 17:41:51 PST
All reviewed patches have been landed.  Closing bug.
Comment 19 Radar WebKit Bug Importer 2019-01-15 17:42:30 PST
<rdar://problem/47303601>
Comment 20 Saam Barati 2019-01-20 12:42:18 PST
Results are:
1% faster Speedometer 2
1% faster ARES6
0-1% faster JetStream2
1% slower JetStream