Bug 133395 – Web Inspector: debugger should be able to show variable types

• Component: JavaScriptCore
• Status: RESOLVED FIXED

[Saam Barati]

Record runtime types of objects to show in the web inspector for debugging purposes.

[Saam Barati]

Created attachment 232278 [details]
patch

[Saam Barati]

Work in progress. Just the beginning.

[Saam Barati]

Created attachment 232512 [details]
Continued progress

The beginning of an interface between JSC high fidelity types and the Inspector implemented.

[Saam Barati]

Created attachment 232596 [details]
path

hover for types in inspector. Currenlty crashing in merge

[Saam Barati]

Created attachment 232788 [details]
patch

Refactored persistent data into VM.

[Saam Barati]

Created attachment 233068 [details]
patch

 Re-factored code to now capture all assignments (op_mov and op_put_to_scope). Created Log Data Stucture on CodeBlock to capture information

[Saam Barati]

Created attachment 233208 [details]
patch

refactored log to be in VM

[Filip Pizlo]

Comment on attachment 233208 [details]
patch

You should check the "Patch" checkbox because that enables in-browser pretty-printing of the patch as well as the review UI.

I've checked it by going to the "details" screen.

[Filip Pizlo]

Are the patches you're posting cumulative?

It would be most useful if you posted a patch that squashed all of the changes you've made so far.

[Saam Barati]

Created attachment 233308 [details]
path

Here is the full diff with master, not the incremental diffs (including all the whitespace issues). 
(I tried to manually edit out whitespace issues in the diff last night but the patch didn't apply. Will try again).

[Filip Pizlo]

(In reply to comment #10)
> Created an attachment (id=233308) [details]
> path
> 
> Here is the full diff with master, not the incremental diffs (including all the whitespace issues). 
> (I tried to manually edit out whitespace issues in the diff last night but the patch didn't apply. Will try again).

You should check the "Patch" checkbox when uploading patches.

[Filip Pizlo]

Comment on attachment 233208 [details]
patch

You should also obsolete old patches when uploading new ones.  Checking the "patch" checkbox when uploading a new patch makes this easy - it'll automatically ask you if you want to obsolete old patches.

[Filip Pizlo]

Comment on attachment 233308 [details]
path

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

Looks like you've got a lot of reverting to do! ;-)

> Source/JavaScriptCore/JavaScriptCore.xcodeproj/project.pbxproj.orig:13
> +// !$*UTF8*$!
> +{
> +	archiveVersion = 1;
> +	classes = {
> +	};
> +	objectVersion = 46;
> +	objects = {
> +
> +/* Begin PBXAggregateTarget section */
> +		0F4680A914BA7FD900BFE272 /* LLInt Offsets */ = {
> +			isa = PBXAggregateTarget;
> +			buildConfigurationList = 0F4680AC14BA7FD900BFE272 /* Build configuration list for PBXAggregateTarget "LLInt Offsets" */;
> +			buildPhases = (

I don't think you meant to add this file.

> Source/JavaScriptCore/bytecode/BytecodeUseDef.h:64
> -    case op_create_activation: 
> +    case op_create_activation:

Revert.

> Source/JavaScriptCore/bytecode/CodeBlock.cpp:115
> -    
> +

Revert.

> Source/JavaScriptCore/bytecode/CodeBlock.cpp:293
> -    
> +

Revert.

> Source/JavaScriptCore/bytecode/CodeBlock.cpp:296
> -    
> +
>      PropertyOffset offset = structure->getConcurrently(exec->vm(), ident.impl());

Revert.

> Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp:1492
> -    RefPtr<RegisterID> func = newTemporary(); 
> +    RefPtr<RegisterID> func = newTemporary();

Revert.

> Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp:1504
> -    emitOpcode(op_create_this); 
> -    instructions().append(m_thisRegister.index()); 
> -    instructions().append(func->index()); 
> +    emitOpcode(op_create_this);
> +    instructions().append(m_thisRegister.index());
> +    instructions().append(func->index());

Revert.

[Saam Barati]

Created attachment 233311 [details]
patch

Clean patch with white space issues fixed.

[Saam Barati]

Created attachment 233394 [details]
patch

2014-06-19  Saam Barati  <[e-mail]>

        Increase the amount of type information the VM gathers when directed
        to do so. This initial commit is working towards the goal of
        capturing, and then showing (via the Web Inspector) type information for all
        assignment and load operations. This patch doesn't have the feature fully 
        implemented, but it ensures the VM has no performance regressions
        unless the feature is specifically turned on.
        https://bugs.webkit.org/show_bug.cgi?id=133395

        Reviewed by NOBODY (OOPS!).

        * JavaScriptCore.xcodeproj/project.pbxproj:
        * bytecode/BytecodeList.json:
        * bytecode/BytecodeUseDef.h:
        (JSC::computeUsesForBytecodeOffset):
        (JSC::computeDefsForBytecodeOffset):
        * bytecode/CodeBlock.cpp:
        (JSC::CodeBlock::dumpBytecode):
        (JSC::CodeBlock::CodeBlock):
        (JSC::CodeBlock::finalizeUnconditionally):
        (JSC::CodeBlock::nameForRegister):
        * bytecode/CodeBlock.h:
        * bytecode/Instruction.h:
        * bytecode/TypeLocation.h: Added.
        (JSC::TypeLocation::TypeLocation):
        (JSC::TypeLocation::~TypeLocation):
        * bytecompiler/BytecodeGenerator.cpp:
        (JSC::BytecodeGenerator::emitMove):
        (JSC::BytecodeGenerator::emitProfileTypesWithHighFidelity):
        (JSC::BytecodeGenerator::emitPutToScope):
        (JSC::BytecodeGenerator::emitPutById):
        * bytecompiler/BytecodeGenerator.h:
        (JSC::BytecodeGenerator::isGeneratingHighFidelityTypeProfiling):
        * bytecompiler/NodesCodegen.cpp:
        (JSC::PostfixNode::emitResolve):
        (JSC::PrefixNode::emitResolve):
        (JSC::ReadModifyResolveNode::emitBytecode):
        (JSC::AssignResolveNode::emitBytecode):
        (JSC::ConstDeclNode::emitCodeSingle):
        (JSC::ForInNode::emitBytecode):
        * heap/Heap.cpp:
        (JSC::Heap::collect):
        * inspector/agents/InspectorRuntimeAgent.cpp:
        (Inspector::InspectorRuntimeAgent::getRuntimeTypeForVariableInTextRange):
        * inspector/agents/InspectorRuntimeAgent.h:
        * inspector/protocol/Runtime.json:
        * jsc.cpp:
        (GlobalObject::finishCreation):
        (functionDumpTypesForAllVariables):
        * llint/LLIntSlowPaths.cpp:
        (JSC::LLInt::LLINT_SLOW_PATH_DECL):
        * llint/LLIntSlowPaths.h:
        * llint/LowLevelInterpreter.asm:
        * runtime/HighFidelityLog.cpp: Added.
        (JSC::HighFidelityLog::initializeHighFidelityLog):
        (JSC::HighFidelityLog::~HighFidelityLog):
        (JSC::HighFidelityLog::recordTypeInformationForLocation):
        (JSC::HighFidelityLog::processHighFidelityLog):
        * runtime/HighFidelityLog.h: Added.
        (JSC::HighFidelityLog::HighFidelityLog):
        * runtime/HighFidelityTypeProfiler.cpp: Added.
        (JSC::HighFidelityTypeProfiler::getTypesForVariableInRange):
        (JSC::HighFidelityTypeProfiler::getGlobalTypesForVariableInRange):
        (JSC::HighFidelityTypeProfiler::getLocalTypesForVariableInRange):
        (JSC::HighFidelityTypeProfiler::mergeInTypesForLocation):
        (JSC::HighFidelityTypeProfiler::getLocationBasedHash):
        * runtime/HighFidelityTypeProfiler.h: Added.
        * runtime/Options.h:
        * runtime/Structure.cpp:
        (JSC::Structure::toStructureShape):
        * runtime/Structure.h:
        * runtime/SymbolTable.cpp:
        (JSC::SymbolTable::SymbolTable):
        (JSC::SymbolTable::cloneCapturedNames):
        (JSC::SymbolTable::uniqueIDForVariable):
        (JSC::SymbolTable::uniqueIDForRegister):
        * runtime/SymbolTable.h:
        (JSC::SymbolTable::add):
        (JSC::SymbolTable::set):
        * runtime/TypeSet.cpp: Added.
        (JSC::TypeSet::TypeSet):
        (JSC::TypeSet::getRuntimeTypeForValue):
        (JSC::TypeSet::addTypeForValue):
        (JSC::TypeSet::mergeWithTypeSet):
        (JSC::TypeSet::removeDuplicatesInStructureHistory):
        (JSC::TypeSet::seenTypes):
        (JSC::TypeSet::dumpSeenTypes):
        (JSC::StructureShape::operator==):
        * runtime/TypeSet.h: Added.
        * runtime/VM.cpp:
        (JSC::VM::VM):
        (JSC::VM::getTypesForVariableInRange):
        (JSC::VM::updateHighFidelityTypeProfileState):
        (JSC::VM::dumpHighFidelityProfilingTypes):
        * runtime/VM.h:
        (JSC::VM::isProfilingTypesWithHighFidelity):
        (JSC::VM::highFidelityLog):
        (JSC::VM::nextLocation):
        (JSC::VM::getNextUniqueVariableID):
        (JSC::VM::getNextUniqueInstructionID):

[Saam Barati]

Created attachment 233486 [details]
patch

* Made processing the log happen on a separate thread when processing is invoked by maxing out the log size.

* Also, re-architected the infrastructure to not make TypeSet copies in the HighFidelityTypeProfiler. This prevents the VM from having to walk over all TypeSets that have changed and then merge them with the profiler. Instead, TypeSet's are all passed around as references, so once the log is processed, the VM is in an updated state. This saves a lot of time when the user specifically asks the VM to update because instead of processing the log, then walking all TypeSets, then doing a has lookup, the VM simply processes the log and does a hash lookup. This cuts out the most expensive part: walking all TypeSets.

[Filip Pizlo]

Comment on attachment 233394 [details]
patch

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

Looks pretty good, just minor nits.

> Source/JavaScriptCore/jsc.cpp:942
> +
> +

Remove

> Source/JavaScriptCore/bytecode/BytecodeUseDef.h:57
> +    //TODO: Should this functor be called with one operand?

Space between // and TODO.

And yes, this should keep the input alive.

> Source/JavaScriptCore/bytecode/CodeBlock.cpp:1926
> +                //TODO: handle other op.type here

Space between // and TODO

> Source/JavaScriptCore/bytecode/CodeBlock.cpp:1933
> +                    //TODO: implement

Ditto.

> Source/JavaScriptCore/bytecode/CodeBlock.cpp:1969
> +            //String name = nameForRegister(virtualRegister);
> +            //printf("LINKING: op_profile_types_with_high_fidelity has uniqueID:'%llu' and name:'%s', line:'%u', column:'%u'\n", globalID, name.ascii().data(), location->line, location->column); 

Remove this.  Or, create a "static const bool verbose = false" and use that as a guard.  Also, don't use printf() - use dataLog() instead.

> Source/JavaScriptCore/bytecode/TypeLocation.h:36
> +    enum HighFidelityGlobalIDFlags {
> +        HighFidelityNeedsUniqueIDGeneration = -1,
> +        HighFidelityNoGlobalIDExists = -2
> +    };

Unindent the enum.

> Source/JavaScriptCore/bytecode/TypeLocation.h:51
> +    //uint64_t globalVariableID;

Remove.

> Source/JavaScriptCore/bytecode/TypeLocation.h:53
> +    int64_t globalVariableID;
> +    uint64_t globalInstructionID;

Prepend "m_" to field names.

> Source/JavaScriptCore/bytecode/TypeLocation.h:54
> +    //size_t unlinkedByteCodeOffset;

Unique.

> Source/JavaScriptCore/bytecode/TypeLocation.h:57
> +    intptr_t sourceID;
> +    unsigned line;
> +    unsigned column;

Prepend "m_" to field names.

> Source/JavaScriptCore/bytecode/TypeLocation.h:58
> +    TypeSet* typeSet;

Use a std::unique_ptr<TypeSet> if this is owned by TypeLocation.

> Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp:1120
> +    //op_profile_types_with_high_fidelity value_reg location hasGlobalID

Turn this comment into a sentence or remove it.

> Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp:1123
> +    instructions().append(0);

You could add a comment at the end of this line saying what this zero means.

> Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp:1129
> +    //instructions().append(offset);

Remove.

> Source/JavaScriptCore/bytecompiler/NodesCodegen.cpp:1884
> +            //TODO: maybe emit expression info???

Space between // and TODO.  Only one ?.  Turn it into a sentence (i.e. capitalize Maybe).

> Source/JavaScriptCore/inspector/agents/InspectorRuntimeAgent.cpp:195
> +                                                                 int in_startLine, int in_startColumn, int in_endLine, int in_endColumn, String* out_types) 

Bad indentation, you should make this line be only 4 spaces indented.

> Source/JavaScriptCore/llint/LLIntSlowPaths.cpp:1471
> +    // put_to_scope_with_profile scope, id, value, ResolveModeAndType, Structure, Operand, TypeLocation*

Same comment as before - turn this into a sentence or remove it.  For example, you could make this say:

// The format of this instruction is: put_to_scope_with_profile scope, id, value, ResolveModeAndType, Structure, Operand, TypeLocation*

> Source/JavaScriptCore/runtime/HighFidelityTypeProfiler.h:2
> + * Copyright (C) 2008, 2014 Apple Inc. All Rights Reserved.

Only say 2014.

> Source/JavaScriptCore/runtime/SymbolTable.h:419
> +        //use -1 as a flag to indicate we need to produce a unique ID because we need the VM to do this. 
> +        //When linking, we can produce this unique ID as we have access to VM

Complete sentences in comments, capitalize the first word in the sentence, and a space between // and use, and between // and When.

> Source/JavaScriptCore/runtime/SymbolTable.h:472
> -    
> +

Revert.

> Source/JavaScriptCore/runtime/SymbolTable.h:485
> -    
> +

Revert.

> Source/JavaScriptCore/runtime/SymbolTable.h:500
> -        
> +

Revert.

> Source/JavaScriptCore/runtime/SymbolTable.h:507
> -    
> +

Revert.

> Source/JavaScriptCore/runtime/SymbolTable.h:522
> -    
> +

Revert.

> Source/JavaScriptCore/runtime/SymbolTable.h:527
> -    
> +

Revert.

> Source/JavaScriptCore/runtime/TypeSet.cpp:63
> +    else if (v.isGetterSetter())
> +        ret = TypeGetterSetter;

Remove.

> Source/JavaScriptCore/runtime/TypeSet.cpp:65
> +    //else if (v.isCustomGetterSetter())
> +    //    ret = TypeCustomGetterSetter;

Remove.

> Source/JavaScriptCore/runtime/TypeSet.cpp:74
> +//structure may be null

Space between // and structure, turn it into a sentence.  Or remove it.

> Source/JavaScriptCore/runtime/TypeSet.cpp:139
> +    if (m_seenTypes & TypeGetterSetter)
> +         seen.append("GetterSetter ");
> +    if (m_seenTypes & TypeCustomGetterSetter)
> +         seen.append("CustomGetterSetter ");
> +    //if (m_seenTypes & TypeObject)
> +    //     seen.append("Object ");

Remove.

> Source/JavaScriptCore/runtime/TypeSet.h:53
> +    class JSValue;
> +    class Structure;
> +
> +    enum RuntimeType {
> +        TypeNothing            = 0x0,
> +        TypeFunction           = 0x1,
> +        TypeUndefined          = 0x2,
> +        TypeNull               = 0x4,
> +        TypeBoolean            = 0x8,
> +        TypeMachineInt         = 0x10,
> +        TypeNumber             = 0x20,
> +        TypeString             = 0x40,
> +        TypePrimitive          = 0x80,
> +        TypeGetterSetter       = 0x100,
> +        TypeCustomGetterSetter = 0x200,
> +        TypeObject             = 0x400,
> +    };
> +

Unindent.

> Source/JavaScriptCore/runtime/TypeSet.h:54
> +

Remove this blank line.

> Source/JavaScriptCore/runtime/TypeSet.h:62
> +

Remove this blank line.

> Source/JavaScriptCore/runtime/VM.cpp:335
> +    //TODO: conditionally allocate all this stuff based on whether the inspector is running

Space between // and TODO.

> Source/JavaScriptCore/runtime/VM.cpp:943
> +    //printf("Getting type for variable given sourceID:'%ld' line:'%u'\n", sourceID, startLine);

Remove or guard with a verbose flag.

> Source/JavaScriptCore/runtime/VM.cpp:971
> +        printf("[Line, Column]::[%u, %u] Local:'%s' Global:'%s'\n", location->line, location->column, 
> +                                               profiler->getLocalTypesForVariableInRange(location->line, location->column, location->line, location->column, "", location->sourceID).ascii().data(),
> +                                               profiler->getGlobalTypesForVariableInRange(location->line, location->column, location->line, location->column, "", location->sourceID).ascii().data());

Use dataLog().

[Saam Barati]

Created attachment 233655 [details]
patch

2014-06-23  Saam Barati  <[e-mail]>

        Increase the amount of type information the VM gathers when directed
        to do so. This initial commit is working towards the goal of
        capturing, and then showing (via the Web Inspector) type information for all
        assignment and load operations. This patch doesn't have the feature fully 
        implemented, but it ensures the VM has no performance regressions
        unless the feature is specifically turned on.
        https://bugs.webkit.org/show_bug.cgi?id=133395

        Reviewed by NOBODY (OOPS!).

        * JavaScriptCore.xcodeproj/project.pbxproj:
        * bytecode/BytecodeList.json:
        * bytecode/BytecodeUseDef.h:
        (JSC::computeUsesForBytecodeOffset):
        (JSC::computeDefsForBytecodeOffset):
        * bytecode/CodeBlock.cpp:
        (JSC::CodeBlock::dumpBytecode):
        (JSC::CodeBlock::CodeBlock):
        (JSC::CodeBlock::finalizeUnconditionally):
        * bytecode/CodeBlock.h:
        * bytecode/Instruction.h:
        * bytecode/TypeLocation.h: Added.
        (JSC::TypeLocation::TypeLocation):
        * bytecompiler/BytecodeGenerator.cpp:
        (JSC::BytecodeGenerator::emitMove):
        (JSC::BytecodeGenerator::emitProfileTypesWithHighFidelity):
        (JSC::BytecodeGenerator::emitPutToScope):
        (JSC::BytecodeGenerator::emitPutById):
        (JSC::BytecodeGenerator::emitPutByVal):
        * bytecompiler/BytecodeGenerator.h:
        (JSC::BytecodeGenerator::isProfilingTypesWithHighFidelity):
        * bytecompiler/NodesCodegen.cpp:
        (JSC::PostfixNode::emitResolve):
        (JSC::PrefixNode::emitResolve):
        (JSC::ReadModifyResolveNode::emitBytecode):
        (JSC::AssignResolveNode::emitBytecode):
        (JSC::ConstDeclNode::emitCodeSingle):
        (JSC::ForInNode::emitBytecode):
        * heap/Heap.cpp:
        (JSC::Heap::collect):
        * inspector/agents/InspectorRuntimeAgent.cpp:
        (Inspector::InspectorRuntimeAgent::getRuntimeTypeForVariableInTextRange):
        * inspector/agents/InspectorRuntimeAgent.h:
        * inspector/protocol/Runtime.json:
        * jsc.cpp:
        (GlobalObject::finishCreation):
        (functionDumpTypesForAllVariables):
        * llint/LLIntSlowPaths.cpp:
        (JSC::LLInt::LLINT_SLOW_PATH_DECL):
        * llint/LLIntSlowPaths.h:
        * llint/LowLevelInterpreter.asm:
        * runtime/HighFidelityLog.cpp: Added.
        (JSC::HighFidelityLog::initializeHighFidelityLog):
        (JSC::HighFidelityLog::~HighFidelityLog):
        (JSC::HighFidelityLog::recordTypeInformationForLocation):
        (JSC::HighFidelityLog::processHighFidelityLog):
        (JSC::HighFidelityLog::actuallyProcessLogThreadFunction):
        * runtime/HighFidelityLog.h: Added.
        (JSC::HighFidelityLog::HighFidelityLog):
        * runtime/HighFidelityTypeProfiler.cpp: Added.
        (JSC::HighFidelityTypeProfiler::getTypesForVariableInRange):
        (JSC::HighFidelityTypeProfiler::getGlobalTypesForVariableInRange):
        (JSC::HighFidelityTypeProfiler::getLocalTypesForVariableInRange):
        (JSC::HighFidelityTypeProfiler::insertNewLocation):
        (JSC::HighFidelityTypeProfiler::getLocationBasedHash):
        * runtime/HighFidelityTypeProfiler.h: Added.
        * runtime/Options.h:
        * runtime/Structure.cpp:
        (JSC::Structure::toStructureShape):
        * runtime/Structure.h:
        * runtime/SymbolTable.cpp:
        (JSC::SymbolTable::SymbolTable):
        (JSC::SymbolTable::cloneCapturedNames):
        (JSC::SymbolTable::uniqueIDForVariable):
        (JSC::SymbolTable::uniqueIDForRegister):
        (JSC::SymbolTable::globalTypeSetForRegister):
        (JSC::SymbolTable::globalTypeSetForVariable):
        * runtime/SymbolTable.h:
        (JSC::SymbolTable::add):
        (JSC::SymbolTable::set):
        * runtime/TypeSet.cpp: Added.
        (JSC::TypeSet::TypeSet):
        (JSC::TypeSet::getRuntimeTypeForValue):
        (JSC::TypeSet::addTypeForValue):
        (JSC::TypeSet::removeDuplicatesInStructureHistory):
        (JSC::TypeSet::seenTypes):
        (JSC::TypeSet::dumpSeenTypes):
        (JSC::StructureShape::StructureShape):
        (JSC::StructureShape::markAsFinal):
        (JSC::StructureShape::addProperty):
        (JSC::StructureShape::propertyHash):
        (JSC::StructureShape::leastUpperBound):
        (JSC::StructureShape::stringRepresentation):
        * runtime/TypeSet.h: Added.
        (JSC::StructureShape::create):
        * runtime/VM.cpp:
        (JSC::VM::VM):
        (JSC::VM::getTypesForVariableInRange):
        (JSC::VM::updateHighFidelityTypeProfileState):
        (JSC::VM::dumpHighFidelityProfilingTypes):
        * runtime/VM.h:
        (JSC::VM::isProfilingTypesWithHighFidelity):
        (JSC::VM::highFidelityLog):
        (JSC::VM::highFidelityTypeProfiler):
        (JSC::VM::nextLocation):
        (JSC::VM::getNextUniqueVariableID):

[Saam Barati]

Created attachment 233658 [details]
patch

2014-06-23  Saam Barati  <[e-mail]>

        Increase the amount of type information the VM gathers when directed
        to do so. This initial commit is working towards the goal of
        capturing, and then showing (via the Web Inspector) type information for all
        assignment and load operations. This patch doesn't have the feature fully 
        implemented, but it ensures the VM has no performance regressions
        unless the feature is specifically turned on.
        https://bugs.webkit.org/show_bug.cgi?id=133395

        Reviewed by NOBODY (OOPS!).

        * JavaScriptCore.xcodeproj/project.pbxproj:
        * bytecode/BytecodeList.json:
        * bytecode/BytecodeUseDef.h:
        (JSC::computeUsesForBytecodeOffset):
        (JSC::computeDefsForBytecodeOffset):
        * bytecode/CodeBlock.cpp:
        (JSC::CodeBlock::dumpBytecode):
        (JSC::CodeBlock::CodeBlock):
        (JSC::CodeBlock::finalizeUnconditionally):
        * bytecode/CodeBlock.h:
        * bytecode/Instruction.h:
        * bytecode/TypeLocation.h: Added.
        (JSC::TypeLocation::TypeLocation):
        * bytecompiler/BytecodeGenerator.cpp:
        (JSC::BytecodeGenerator::emitMove):
        (JSC::BytecodeGenerator::emitProfileTypesWithHighFidelity):
        (JSC::BytecodeGenerator::emitPutToScope):
        (JSC::BytecodeGenerator::emitPutById):
        (JSC::BytecodeGenerator::emitPutByVal):
        * bytecompiler/BytecodeGenerator.h:
        (JSC::BytecodeGenerator::isProfilingTypesWithHighFidelity):
        * bytecompiler/NodesCodegen.cpp:
        (JSC::PostfixNode::emitResolve):
        (JSC::PrefixNode::emitResolve):
        (JSC::ReadModifyResolveNode::emitBytecode):
        (JSC::AssignResolveNode::emitBytecode):
        (JSC::ConstDeclNode::emitCodeSingle):
        (JSC::ForInNode::emitBytecode):
        * heap/Heap.cpp:
        (JSC::Heap::collect):
        * inspector/agents/InspectorRuntimeAgent.cpp:
        (Inspector::InspectorRuntimeAgent::getRuntimeTypeForVariableInTextRange):
        * inspector/agents/InspectorRuntimeAgent.h:
        * inspector/protocol/Runtime.json:
        * jsc.cpp:
        (GlobalObject::finishCreation):
        (functionDumpTypesForAllVariables):
        * llint/LLIntSlowPaths.cpp:
        (JSC::LLInt::LLINT_SLOW_PATH_DECL):
        * llint/LLIntSlowPaths.h:
        * llint/LowLevelInterpreter.asm:
        * runtime/HighFidelityLog.cpp: Added.
        (JSC::HighFidelityLog::initializeHighFidelityLog):
        (JSC::HighFidelityLog::~HighFidelityLog):
        (JSC::HighFidelityLog::recordTypeInformationForLocation):
        (JSC::HighFidelityLog::processHighFidelityLog):
        (JSC::HighFidelityLog::actuallyProcessLogThreadFunction):
        * runtime/HighFidelityLog.h: Added.
        (JSC::HighFidelityLog::HighFidelityLog):
        * runtime/HighFidelityTypeProfiler.cpp: Added.
        (JSC::HighFidelityTypeProfiler::getTypesForVariableInRange):
        (JSC::HighFidelityTypeProfiler::getGlobalTypesForVariableInRange):
        (JSC::HighFidelityTypeProfiler::getLocalTypesForVariableInRange):
        (JSC::HighFidelityTypeProfiler::insertNewLocation):
        (JSC::HighFidelityTypeProfiler::getLocationBasedHash):
        * runtime/HighFidelityTypeProfiler.h: Added.
        * runtime/Options.h:
        * runtime/Structure.cpp:
        (JSC::Structure::toStructureShape):
        * runtime/Structure.h:
        * runtime/SymbolTable.cpp:
        (JSC::SymbolTable::SymbolTable):
        (JSC::SymbolTable::cloneCapturedNames):
        (JSC::SymbolTable::uniqueIDForVariable):
        (JSC::SymbolTable::uniqueIDForRegister):
        (JSC::SymbolTable::globalTypeSetForRegister):
        (JSC::SymbolTable::globalTypeSetForVariable):
        * runtime/SymbolTable.h:
        (JSC::SymbolTable::add):
        (JSC::SymbolTable::set):
        * runtime/TypeSet.cpp: Added.
        (JSC::TypeSet::TypeSet):
        (JSC::TypeSet::getRuntimeTypeForValue):
        (JSC::TypeSet::addTypeForValue):
        (JSC::TypeSet::removeDuplicatesInStructureHistory):
        (JSC::TypeSet::seenTypes):
        (JSC::TypeSet::dumpSeenTypes):
        (JSC::StructureShape::StructureShape):
        (JSC::StructureShape::markAsFinal):
        (JSC::StructureShape::addProperty):
        (JSC::StructureShape::propertyHash):
        (JSC::StructureShape::leastUpperBound):
        (JSC::StructureShape::stringRepresentation):
        * runtime/TypeSet.h: Added.
        (JSC::StructureShape::create):
        * runtime/VM.cpp:
        (JSC::VM::VM):
        (JSC::VM::getTypesForVariableInRange):
        (JSC::VM::updateHighFidelityTypeProfileState):
        (JSC::VM::dumpHighFidelityProfilingTypes):
        * runtime/VM.h:
        (JSC::VM::isProfilingTypesWithHighFidelity):
        (JSC::VM::highFidelityLog):
        (JSC::VM::highFidelityTypeProfiler):
        (JSC::VM::nextLocation):
        (JSC::VM::getNextUniqueVariableID):

[Filip Pizlo]

Comment on attachment 233658 [details]
patch

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

I think I found a bug (std::unique_ptr in TypeLocation for the global set thing) and some obvious performance goofs (the log being an array of Log3Triple*'s instead of Log3Triples, and the use of append() instead of StringBuilder).

> Source/JavaScriptCore/ChangeLog:8
> +        Increase the amount of type information the VM gathers when directed
> +        to do so. This initial commit is working towards the goal of
> +        capturing, and then showing (via the Web Inspector) type information for all
> +        assignment and load operations. This patch doesn't have the feature fully 
> +        implemented, but it ensures the VM has no performance regressions
> +        unless the feature is specifically turned on.

Up here you should just put the bug title.

> Source/JavaScriptCore/ChangeLog:13
> +        Reviewed by NOBODY (OOPS!).
> +
> +        * JavaScriptCore.xcodeproj/project.pbxproj:

In between the "Reviewed by..." and the list of changed files, you should put the text that you have up above.

> Source/JavaScriptCore/bytecode/TypeLocation.h:52
> +    std::unique_ptr<TypeSet> m_globalTypeSet;

Should this *really* be a unique_ptr?  Is it ever possible for the thing that globalTypeSet points to to outlive the TypeLocation?  Is it ever possible for other things to point at the TypeSet pointed to by m_globalTypeSet?  If you said "yes" to either question, then this shouldn't be a unique_ptr.

> Source/JavaScriptCore/llint/LLIntSlowPaths.cpp:194
> +#define LLINT_PUT_TO_SCOPE_COMMON() \
> +    LLINT_BEGIN(); \
> +    CodeBlock* codeBlock = exec->codeBlock(); \
> +    const Identifier& ident = codeBlock->identifier(pc[2].u.operand); \
> +    JSObject* scope = jsCast<JSObject*>(LLINT_OP(1).jsValue()); \
> +    JSValue value = LLINT_OP_C(3).jsValue(); \
> +    ResolveModeAndType modeAndType = ResolveModeAndType(pc[4].u.operand); \
> + \
> +    if (modeAndType.mode() == ThrowIfNotFound && !scope->hasProperty(exec, ident)) \
> +        LLINT_THROW(createUndefinedVariableError(exec, ident)); \
> + \
> +    PutPropertySlot slot(scope, codeBlock->isStrictMode()); \
> +    scope->methodTable()->put(scope, exec, ident, value, slot); \
> + \
> +    /* Covers implicit globals. Since they don't exist until they first execute, we didn't know how to cache them at compile time.*/  \
> +    if (modeAndType.type() == GlobalProperty || modeAndType.type() == GlobalPropertyWithVarInjectionChecks) { \
> +        if (slot.isCacheablePut() && slot.base() == scope && scope->structure()->propertyAccessesAreCacheable()) { \
> +            ConcurrentJITLocker locker(codeBlock->m_lock); \
> +            pc[5].u.structure.set(exec->vm(), codeBlock->ownerExecutable(), scope->structure()); \
> +            pc[6].u.operand = slot.cachedOffset(); \
> +        } \
> +    } \
> +

Does this really have to be a macro?  Could this be a helper function?

> Source/JavaScriptCore/runtime/HighFidelityLog.cpp:46
> +    m_logStartPtr = (Log3Tuple**) malloc(sizeof(Log3Tuple*) * m_highFidelityLogSize);
> +    m_nextBuffer = (Log3Tuple**) malloc(sizeof(Log3Tuple*) * m_highFidelityLogSize);

Don't use malloc().  At a minimum, use fastMalloc().  Even better yet, use C++ allocation, like new Log3Tuple[m_highFidelityLogSize].

I also don't agree with your decision to use an array of pointers to a bunch of Log3Tuple instances.  That's just an extra unnecessary indirection.  You could instead just have an array of Log3Tuples (i.e. a Log3Tuple* instead of Log3Tuple**).  This will save memory (fewer pointers and less allocator per-object overhead) and execution time (no need for an extra load to get to the actual Log3Tuple).

Finally, wouldn't it be better to say LogTriple or LogEntry instead of Log3Tuple?

> Source/JavaScriptCore/runtime/HighFidelityTypeProfiler.cpp:53
> +    auto iter2 = m_globalIDMap.find(iter->value);
> +    auto end2 = m_globalIDMap.end();
> +    if (iter2 == end2)

You should come up with better names than iter2 and end2.

> Source/JavaScriptCore/runtime/HighFidelityTypeProfiler.h:43
> +    WTF::String getTypesForVariableInRange(unsigned startLine, unsigned startColumn, unsigned endLine, unsigned endColumn, const WTF::String& variableName, intptr_t sourceID);
> +    WTF::String getGlobalTypesForVariableInRange(unsigned startLine, unsigned startColumn, unsigned endLine, unsigned endColumn, const WTF::String& variableName, intptr_t sourceID);
> +    WTF::String getLocalTypesForVariableInRange(unsigned startLine, unsigned startColumn, unsigned endLine, unsigned endColumn, const WTF::String& variableName, intptr_t sourceID);
> +    void insertNewLocation(TypeLocation*);

No need to say WTF::String.  You can just say String.

> Source/JavaScriptCore/runtime/TypeSet.cpp:179
> +    m_propertyHash = std::make_unique<String>();
> +    m_propertyHash->append(":");
> +    for (auto iter = m_fields.begin(), end = m_fields.end(); iter != end; ++iter) {
> +        String property = String(iter->key);
> +        property.replace(":", "\\:"); // Ensure that hash({"foo:", "bar"}) != hash({"foo", ":bar"}) because we're using colons as a separator and colons are legal characters in field names in JS.
> +        m_propertyHash->append(property);
> +    }

I suspect that you should use StringBuilder here.

> Source/JavaScriptCore/runtime/TypeSet.cpp:209
> +    String lub("");
> +
> +    if (!shapes->size())
> +        return lub;
> +     
> +    RefPtr<StructureShape> origin = shapes->at(0);
> +    lub.append("{");
> +    for (auto iter = origin->m_fields.begin(), end = origin->m_fields.end(); iter != end; ++iter) {
> +        bool shouldAdd = true;
> +        for (size_t i = 1, size = shapes->size(); i < size; i++) {
> +            // If all other Shapes have the same field as origin, add it to the least upper bound.
> +            if (!shapes->at(i)->m_fields.contains(iter->key)) {
> +                shouldAdd = false;
> +                break;
> +            }
> +        }
> +        if (shouldAdd)
> +            lub.append(String(iter->key.get()) + String(", "));
> +    }
> +
> +    if (lub.length() >= 3)
> +        lub = lub.left(lub.length() - 2); // Remove the trailing ', '
> +
> +    lub.append("}");

StringBuilder again, please.

[Saam Barati]

Created attachment 233743 [details]
patch

2014-06-24  Saam Barati  <[e-mail]>

        Web Inspector: debugger should be able to show variable types
        https://bugs.webkit.org/show_bug.cgi?id=133395

        Reviewed by NOBODY (OOPS!).

        Increase the amount of type information the VM gathers when directed
        to do so. This initial commit is working towards the goal of
        capturing, and then showing (via the Web Inspector) type information for all
        assignment and load operations. This patch doesn't have the feature fully 
        implemented, but it ensures the VM has no performance regressions
        unless the feature is specifically turned on.

        * JavaScriptCore.xcodeproj/project.pbxproj:
        * bytecode/BytecodeList.json:
        * bytecode/BytecodeUseDef.h:
        (JSC::computeUsesForBytecodeOffset):
        (JSC::computeDefsForBytecodeOffset):
        * bytecode/CodeBlock.cpp:
        (JSC::CodeBlock::dumpBytecode):
        (JSC::CodeBlock::CodeBlock):
        (JSC::CodeBlock::finalizeUnconditionally):
        * bytecode/CodeBlock.h:
        * bytecode/Instruction.h:
        * bytecode/TypeLocation.h: Added.
        (JSC::TypeLocation::TypeLocation):
        * bytecompiler/BytecodeGenerator.cpp:
        (JSC::BytecodeGenerator::emitMove):
        (JSC::BytecodeGenerator::emitProfileTypesWithHighFidelity):
        (JSC::BytecodeGenerator::emitPutToScope):
        (JSC::BytecodeGenerator::emitPutById):
        (JSC::BytecodeGenerator::emitPutByVal):
        * bytecompiler/BytecodeGenerator.h:
        (JSC::BytecodeGenerator::isProfilingTypesWithHighFidelity):
        * bytecompiler/NodesCodegen.cpp:
        (JSC::PostfixNode::emitResolve):
        (JSC::PrefixNode::emitResolve):
        (JSC::ReadModifyResolveNode::emitBytecode):
        (JSC::AssignResolveNode::emitBytecode):
        (JSC::ConstDeclNode::emitCodeSingle):
        (JSC::ForInNode::emitBytecode):
        * heap/Heap.cpp:
        (JSC::Heap::collect):
        * inspector/agents/InspectorRuntimeAgent.cpp:
        (Inspector::InspectorRuntimeAgent::getRuntimeTypeForVariableInTextRange):
        * inspector/agents/InspectorRuntimeAgent.h:
        * inspector/protocol/Runtime.json:
        * jsc.cpp:
        (GlobalObject::finishCreation):
        (functionDumpTypesForAllVariables):
        * llint/LLIntSlowPaths.cpp:
        (JSC::LLInt::LLINT_SLOW_PATH_DECL):
        (JSC::LLInt::putToScopeCommon):
        * llint/LLIntSlowPaths.h:
        * llint/LowLevelInterpreter.asm:
        * runtime/HighFidelityLog.cpp: Added.
        (JSC::HighFidelityLog::initializeHighFidelityLog):
        (JSC::HighFidelityLog::~HighFidelityLog):
        (JSC::HighFidelityLog::recordTypeInformationForLocation):
        (JSC::HighFidelityLog::processHighFidelityLog):
        (JSC::HighFidelityLog::actuallyProcessLogThreadFunction):
        * runtime/HighFidelityLog.h: Added.
        (JSC::HighFidelityLog::HighFidelityLog):
        * runtime/HighFidelityTypeProfiler.cpp: Added.
        (JSC::HighFidelityTypeProfiler::getTypesForVariableInRange):
        (JSC::HighFidelityTypeProfiler::getGlobalTypesForVariableInRange):
        (JSC::HighFidelityTypeProfiler::getLocalTypesForVariableInRange):
        (JSC::HighFidelityTypeProfiler::insertNewLocation):
        (JSC::HighFidelityTypeProfiler::getLocationBasedHash):
        * runtime/HighFidelityTypeProfiler.h: Added.
        * runtime/Options.h:
        * runtime/Structure.cpp:
        (JSC::Structure::toStructureShape):
        * runtime/Structure.h:
        * runtime/SymbolTable.cpp:
        (JSC::SymbolTable::SymbolTable):
        (JSC::SymbolTable::cloneCapturedNames):
        (JSC::SymbolTable::uniqueIDForVariable):
        (JSC::SymbolTable::uniqueIDForRegister):
        (JSC::SymbolTable::globalTypeSetForRegister):
        (JSC::SymbolTable::globalTypeSetForVariable):
        * runtime/SymbolTable.h:
        (JSC::SymbolTable::add):
        (JSC::SymbolTable::set):
        * runtime/TypeSet.cpp: Added.
        (JSC::TypeSet::TypeSet):
        (JSC::TypeSet::getRuntimeTypeForValue):
        (JSC::TypeSet::addTypeForValue):
        (JSC::TypeSet::removeDuplicatesInStructureHistory):
        (JSC::TypeSet::seenTypes):
        (JSC::TypeSet::dumpSeenTypes):
        (JSC::StructureShape::StructureShape):
        (JSC::StructureShape::markAsFinal):
        (JSC::StructureShape::addProperty):
        (JSC::StructureShape::propertyHash):
        (JSC::StructureShape::leastUpperBound):
        (JSC::StructureShape::stringRepresentation):
        * runtime/TypeSet.h: Added.
        (JSC::StructureShape::create):
        (JSC::TypeSet::create):
        * runtime/VM.cpp:
        (JSC::VM::VM):
        (JSC::VM::getTypesForVariableInRange):
        (JSC::VM::updateHighFidelityTypeProfileState):
        (JSC::VM::dumpHighFidelityProfilingTypes):
        * runtime/VM.h:
        (JSC::VM::isProfilingTypesWithHighFidelity):
        (JSC::VM::highFidelityLog):
        (JSC::VM::highFidelityTypeProfiler):
        (JSC::VM::nextLocation):
        (JSC::VM::getNextUniqueVariableID):

[Filip Pizlo]

Comment on attachment 233743 [details]
patch

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

> Source/JavaScriptCore/runtime/HighFidelityLog.h:39
> +    class TypeLocation;

Unindent this.

> Source/JavaScriptCore/runtime/HighFidelityTypeProfiler.h:48
> +    typedef HashMap<String, RefPtr<TypeSet>> GlobalLocationMap; // (for now, hack it and say Map:"ID:Line" => TypeSet. Obviously, this assumes all assignments are on discrete lines).

This is pretty shady.  The key should be a struct.  Since that takes effort and this code uses few cycles, you can use std::unordered_map.

> Source/JavaScriptCore/runtime/HighFidelityTypeProfiler.h:50
> +    typedef HashMap<String, int64_t> GlobalLocationToGlobalIDMap;

Ditto.

> Source/JavaScriptCore/runtime/Structure.cpp:1071
> +        for (; iter != end; ++iter) {
> +            shape->addProperty(iter->key);
> +        }

No need for { and } if the body is just one line.  You can just say:

for (; iter != end; ++iter)
    shape->addProperty(iter->key);

[Saam Barati]

Created attachment 233760 [details]
patch

2014-06-24  Saam Barati  <[e-mail]>

        Web Inspector: debugger should be able to show variable types
        https://bugs.webkit.org/show_bug.cgi?id=133395

        Reviewed by NOBODY (OOPS!).

        Increase the amount of type information the VM gathers when directed
        to do so. This initial commit is working towards the goal of
        capturing, and then showing (via the Web Inspector) type information for all
        assignment and load operations. This patch doesn't have the feature fully 
        implemented, but it ensures the VM has no performance regressions
        unless the feature is specifically turned on.

        * JavaScriptCore.xcodeproj/project.pbxproj:
        * bytecode/BytecodeList.json:
        * bytecode/BytecodeUseDef.h:
        (JSC::computeUsesForBytecodeOffset):
        (JSC::computeDefsForBytecodeOffset):
        * bytecode/CodeBlock.cpp:
        (JSC::CodeBlock::dumpBytecode):
        (JSC::CodeBlock::CodeBlock):
        (JSC::CodeBlock::finalizeUnconditionally):
        * bytecode/CodeBlock.h:
        * bytecode/Instruction.h:
        * bytecode/TypeLocation.h: Added.
        (JSC::TypeLocation::TypeLocation):
        * bytecompiler/BytecodeGenerator.cpp:
        (JSC::BytecodeGenerator::emitMove):
        (JSC::BytecodeGenerator::emitProfileTypesWithHighFidelity):
        (JSC::BytecodeGenerator::emitPutToScope):
        (JSC::BytecodeGenerator::emitPutById):
        (JSC::BytecodeGenerator::emitPutByVal):
        * bytecompiler/BytecodeGenerator.h:
        (JSC::BytecodeGenerator::isProfilingTypesWithHighFidelity):
        * bytecompiler/NodesCodegen.cpp:
        (JSC::PostfixNode::emitResolve):
        (JSC::PrefixNode::emitResolve):
        (JSC::ReadModifyResolveNode::emitBytecode):
        (JSC::AssignResolveNode::emitBytecode):
        (JSC::ConstDeclNode::emitCodeSingle):
        (JSC::ForInNode::emitBytecode):
        * heap/Heap.cpp:
        (JSC::Heap::collect):
        * inspector/agents/InspectorRuntimeAgent.cpp:
        (Inspector::InspectorRuntimeAgent::getRuntimeTypeForVariableInTextRange):
        * inspector/agents/InspectorRuntimeAgent.h:
        * inspector/protocol/Runtime.json:
        * jsc.cpp:
        (GlobalObject::finishCreation):
        (functionDumpTypesForAllVariables):
        * llint/LLIntSlowPaths.cpp:
        (JSC::LLInt::LLINT_SLOW_PATH_DECL):
        (JSC::LLInt::putToScopeCommon):
        * llint/LLIntSlowPaths.h:
        * llint/LowLevelInterpreter.asm:
        * runtime/HighFidelityLog.cpp: Added.
        (JSC::HighFidelityLog::initializeHighFidelityLog):
        (JSC::HighFidelityLog::~HighFidelityLog):
        (JSC::HighFidelityLog::recordTypeInformationForLocation):
        (JSC::HighFidelityLog::processHighFidelityLog):
        (JSC::HighFidelityLog::actuallyProcessLogThreadFunction):
        * runtime/HighFidelityLog.h: Added.
        (JSC::HighFidelityLog::HighFidelityLog):
        * runtime/HighFidelityTypeProfiler.cpp: Added.
        (JSC::HighFidelityTypeProfiler::getTypesForVariableInRange):
        (JSC::HighFidelityTypeProfiler::getGlobalTypesForVariableInRange):
        (JSC::HighFidelityTypeProfiler::getLocalTypesForVariableInRange):
        (JSC::HighFidelityTypeProfiler::insertNewLocation):
        (JSC::HighFidelityTypeProfiler::getLocationBasedHash):
        * runtime/HighFidelityTypeProfiler.h: Added.
        * runtime/Options.h:
        * runtime/Structure.cpp:
        (JSC::Structure::toStructureShape):
        * runtime/Structure.h:
        * runtime/SymbolTable.cpp:
        (JSC::SymbolTable::SymbolTable):
        (JSC::SymbolTable::cloneCapturedNames):
        (JSC::SymbolTable::uniqueIDForVariable):
        (JSC::SymbolTable::uniqueIDForRegister):
        (JSC::SymbolTable::globalTypeSetForRegister):
        (JSC::SymbolTable::globalTypeSetForVariable):
        * runtime/SymbolTable.h:
        (JSC::SymbolTable::add):
        (JSC::SymbolTable::set):
        * runtime/TypeSet.cpp: Added.
        (JSC::TypeSet::TypeSet):
        (JSC::TypeSet::getRuntimeTypeForValue):
        (JSC::TypeSet::addTypeForValue):
        (JSC::TypeSet::removeDuplicatesInStructureHistory):
        (JSC::TypeSet::seenTypes):
        (JSC::TypeSet::dumpSeenTypes):
        (JSC::StructureShape::StructureShape):
        (JSC::StructureShape::markAsFinal):
        (JSC::StructureShape::addProperty):
        (JSC::StructureShape::propertyHash):
        (JSC::StructureShape::leastUpperBound):
        (JSC::StructureShape::stringRepresentation):
        * runtime/TypeSet.h: Added.
        (JSC::StructureShape::create):
        (JSC::TypeSet::create):
        * runtime/VM.cpp:
        (JSC::VM::VM):
        (JSC::VM::getTypesForVariableInRange):
        (JSC::VM::updateHighFidelityTypeProfileState):
        (JSC::VM::dumpHighFidelityProfilingTypes):
        * runtime/VM.h:
        (JSC::VM::isProfilingTypesWithHighFidelity):
        (JSC::VM::highFidelityLog):
        (JSC::VM::highFidelityTypeProfiler):
        (JSC::VM::nextLocation):
        (JSC::VM::getNextUniqueVariableID):

[Saam Barati]

Created attachment 233830 [details]
patch

2014-06-24  Saam Barati  <[e-mail]>

        Web Inspector: debugger should be able to show variable types
        https://bugs.webkit.org/show_bug.cgi?id=133395

        Reviewed by NOBODY (OOPS!).

        Increase the amount of type information the VM gathers when directed
        to do so. This initial commit is working towards the goal of
        capturing, and then showing (via the Web Inspector) type information for all
        assignment and load operations. This patch doesn't have the feature fully 
        implemented, but it ensures the VM has no performance regressions
        unless the feature is specifically turned on.

        * JavaScriptCore.xcodeproj/project.pbxproj:
        * bytecode/BytecodeList.json:
        * bytecode/BytecodeUseDef.h:
        (JSC::computeUsesForBytecodeOffset):
        (JSC::computeDefsForBytecodeOffset):
        * bytecode/CodeBlock.cpp:
        (JSC::CodeBlock::dumpBytecode):
        (JSC::CodeBlock::CodeBlock):
        (JSC::CodeBlock::finalizeUnconditionally):
        * bytecode/CodeBlock.h:
        * bytecode/Instruction.h:
        * bytecode/TypeLocation.h: Added.
        (JSC::TypeLocation::TypeLocation):
        * bytecompiler/BytecodeGenerator.cpp:
        (JSC::BytecodeGenerator::emitMove):
        (JSC::BytecodeGenerator::emitProfileTypesWithHighFidelity):
        (JSC::BytecodeGenerator::emitPutToScope):
        (JSC::BytecodeGenerator::emitPutById):
        (JSC::BytecodeGenerator::emitPutByVal):
        * bytecompiler/BytecodeGenerator.h:
        (JSC::BytecodeGenerator::isProfilingTypesWithHighFidelity):
        * bytecompiler/NodesCodegen.cpp:
        (JSC::PostfixNode::emitResolve):
        (JSC::PrefixNode::emitResolve):
        (JSC::ReadModifyResolveNode::emitBytecode):
        (JSC::AssignResolveNode::emitBytecode):
        (JSC::ConstDeclNode::emitCodeSingle):
        (JSC::ForInNode::emitBytecode):
        * heap/Heap.cpp:
        (JSC::Heap::collect):
        * inspector/agents/InspectorRuntimeAgent.cpp:
        (Inspector::InspectorRuntimeAgent::getRuntimeTypeForVariableInTextRange):
        * inspector/agents/InspectorRuntimeAgent.h:
        * inspector/protocol/Runtime.json:
        * jsc.cpp:
        (GlobalObject::finishCreation):
        (functionDumpTypesForAllVariables):
        * llint/LLIntSlowPaths.cpp:
        (JSC::LLInt::LLINT_SLOW_PATH_DECL):
        (JSC::LLInt::putToScopeCommon):
        * llint/LLIntSlowPaths.h:
        * llint/LowLevelInterpreter.asm:
        * runtime/HighFidelityLog.cpp: Added.
        (JSC::HighFidelityLog::initializeHighFidelityLog):
        (JSC::HighFidelityLog::~HighFidelityLog):
        (JSC::HighFidelityLog::recordTypeInformationForLocation):
        (JSC::HighFidelityLog::processHighFidelityLog):
        (JSC::HighFidelityLog::actuallyProcessLogThreadFunction):
        * runtime/HighFidelityLog.h: Added.
        (JSC::HighFidelityLog::HighFidelityLog):
        * runtime/HighFidelityTypeProfiler.cpp: Added.
        (JSC::HighFidelityTypeProfiler::getTypesForVariableInRange):
        (JSC::HighFidelityTypeProfiler::getGlobalTypesForVariableInRange):
        (JSC::HighFidelityTypeProfiler::getLocalTypesForVariableInRange):
        (JSC::HighFidelityTypeProfiler::insertNewLocation):
        (JSC::HighFidelityTypeProfiler::getLocationBasedHash):
        * runtime/HighFidelityTypeProfiler.h: Added.
        * runtime/Options.h:
        * runtime/Structure.cpp:
        (JSC::Structure::toStructureShape):
        * runtime/Structure.h:
        * runtime/SymbolTable.cpp:
        (JSC::SymbolTable::SymbolTable):
        (JSC::SymbolTable::cloneCapturedNames):
        (JSC::SymbolTable::uniqueIDForVariable):
        (JSC::SymbolTable::uniqueIDForRegister):
        (JSC::SymbolTable::globalTypeSetForRegister):
        (JSC::SymbolTable::globalTypeSetForVariable):
        * runtime/SymbolTable.h:
        (JSC::SymbolTable::add):
        (JSC::SymbolTable::set):
        * runtime/TypeSet.cpp: Added.
        (JSC::TypeSet::TypeSet):
        (JSC::TypeSet::getRuntimeTypeForValue):
        (JSC::TypeSet::addTypeForValue):
        (JSC::TypeSet::removeDuplicatesInStructureHistory):
        (JSC::TypeSet::seenTypes):
        (JSC::TypeSet::dumpSeenTypes):
        (JSC::StructureShape::StructureShape):
        (JSC::StructureShape::markAsFinal):
        (JSC::StructureShape::addProperty):
        (JSC::StructureShape::propertyHash):
        (JSC::StructureShape::leastUpperBound):
        (JSC::StructureShape::stringRepresentation):
        * runtime/TypeSet.h: Added.
        (JSC::StructureShape::create):
        (JSC::TypeSet::create):
        * runtime/VM.cpp:
        (JSC::VM::VM):
        (JSC::VM::getTypesForVariableInRange):
        (JSC::VM::updateHighFidelityTypeProfileState):
        (JSC::VM::dumpHighFidelityProfilingTypes):
        * runtime/VM.h:
        (JSC::VM::isProfilingTypesWithHighFidelity):
        (JSC::VM::highFidelityLog):
        (JSC::VM::highFidelityTypeProfiler):
        (JSC::VM::nextLocation):
        (JSC::VM::getNextUniqueVariableID):

[Filip Pizlo]

Comment on attachment 233830 [details]
patch

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

I will land this.

> Source/JavaScriptCore/runtime/HighFidelityLog.cpp:91
> +    LogEntry* temp = m_logStartPtr;
> +    m_logStartPtr = m_nextBuffer;
> +    m_nextBuffer = temp;
> +    

std::swap()

[Filip Pizlo]

Landed in http://trac.webkit.org/changeset/170490

[Saam Barati]

Created attachment 234049 [details]
patch

This patch re-works locations from line/column tuple to a start/end character offset.

Also, the profile_types_with_high_fidelity opcode is emitted in more places:
- At the call site of functions after they return (to capture their return values)
- At the beginning of the function that is called to profile its arguments.
- At getById/getByValue sites.

A side table of assignmentExpressionInfo is created (though the word assignment is misleading, it should really be highFidelityProfileExpressionInfo; I will rename this in the next patch).

[Saam Barati]

Created attachment 234137 [details]
patch

2014-06-30  Saam Barati  <[e-mail]>

        Web Inspector: debugger should be able to show variable types
        https://bugs.webkit.org/show_bug.cgi?id=133395

        Reviewed by NOBODY (OOPS!).

        More operations are now being recorded by the profile_types_with_high_fidelity 
        opcode. Specifically: function parameters, function return values,
        function 'this' value, get_by_id, get_by_value, function return values
        at the call site. Added more flags to the profile_types_with_high_fidelity
        opcode so more focused tasks can take place when the instruction is
        being linked in CodeBlock. Re-worked the type profiler to search 
        through character offset ranges when asked for the type of an expression
        at a given offset.

        * bytecode/CodeBlock.cpp:
        (JSC::CodeBlock::CodeBlock):
        * bytecode/CodeBlock.h:
        (JSC::CodeBlock::returnStatementTypeSet):
        * bytecode/TypeLocation.h:
        * bytecode/UnlinkedCodeBlock.cpp:
        (JSC::UnlinkedCodeBlock::highFidelityTypeProfileExpressionInfoForBytecodeOffset):
        (JSC::UnlinkedCodeBlock::addHighFidelityTypeProfileExpressionInfo):
        * bytecode/UnlinkedCodeBlock.h:
        * bytecompiler/BytecodeGenerator.cpp:
        (JSC::BytecodeGenerator::emitMove):
        (JSC::BytecodeGenerator::emitProfileTypesWithHighFidelity):
        (JSC::BytecodeGenerator::emitPutById):
        (JSC::BytecodeGenerator::emitPutByVal):
        * bytecompiler/BytecodeGenerator.h:
        (JSC::BytecodeGenerator::emitHighFidelityTypeProfilingExpressionInfo):
        * bytecompiler/NodesCodegen.cpp:
        (JSC::BracketAccessorNode::emitBytecode):
        (JSC::DotAccessorNode::emitBytecode):
        (JSC::FunctionCallValueNode::emitBytecode):
        (JSC::FunctionCallResolveNode::emitBytecode):
        (JSC::FunctionCallBracketNode::emitBytecode):
        (JSC::FunctionCallDotNode::emitBytecode):
        (JSC::CallFunctionCallDotNode::emitBytecode):
        (JSC::ApplyFunctionCallDotNode::emitBytecode):
        (JSC::PostfixNode::emitResolve):
        (JSC::PostfixNode::emitBracket):
        (JSC::PostfixNode::emitDot):
        (JSC::PrefixNode::emitResolve):
        (JSC::PrefixNode::emitBracket):
        (JSC::PrefixNode::emitDot):
        (JSC::ReadModifyResolveNode::emitBytecode):
        (JSC::AssignResolveNode::emitBytecode):
        (JSC::AssignDotNode::emitBytecode):
        (JSC::ReadModifyDotNode::emitBytecode):
        (JSC::AssignBracketNode::emitBytecode):
        (JSC::ReadModifyBracketNode::emitBytecode):
        (JSC::ReturnNode::emitBytecode):
        (JSC::FunctionBodyNode::emitBytecode):
        (JSC::FuncDeclNode::emitBytecode):
        * inspector/agents/InspectorRuntimeAgent.cpp:
        (Inspector::InspectorRuntimeAgent::getRuntimeTypeForVariableAtOffset):
        (Inspector::InspectorRuntimeAgent::getRuntimeTypeForVariableInTextRange): Deleted.
        * inspector/agents/InspectorRuntimeAgent.h:
        * inspector/protocol/Runtime.json:
        * runtime/HighFidelityLog.cpp:
        (JSC::HighFidelityLog::processHighFidelityLog):
        * runtime/HighFidelityTypeProfiler.cpp:
        (JSC::HighFidelityTypeProfiler::getTypesForVariableInAtOffset):
        (JSC::HighFidelityTypeProfiler::getGlobalTypesForVariableAtOffset):
        (JSC::HighFidelityTypeProfiler::getLocalTypesForVariableAtOffset):
        (JSC::HighFidelityTypeProfiler::insertNewLocation):
        (JSC::HighFidelityTypeProfiler::findLocation):
        (JSC::HighFidelityTypeProfiler::getTypesForVariableInRange): Deleted.
        (JSC::HighFidelityTypeProfiler::getGlobalTypesForVariableInRange): Deleted.
        (JSC::HighFidelityTypeProfiler::getLocalTypesForVariableInRange): Deleted.
        (JSC::HighFidelityTypeProfiler::getLocationBasedHash): Deleted.
        * runtime/HighFidelityTypeProfiler.h:
        (JSC::LocationKey::LocationKey): Deleted.
        (JSC::LocationKey::hash): Deleted.
        (JSC::LocationKey::operator==): Deleted.
        * runtime/VM.cpp:
        (JSC::VM::getTypesForVariableAtOffset):
        (JSC::VM::dumpHighFidelityProfilingTypes):
        (JSC::VM::getTypesForVariableInRange): Deleted.
        * runtime/VM.h:

[Saam Barati]

Created attachment 234237 [details]
patch

2014-07-01  Saam Barati  <[e-mail]>

        Web Inspector: debugger should be able to show variable types
        https://bugs.webkit.org/show_bug.cgi?id=133395

        Reviewed by NOBODY (OOPS!).

        More operations are now being recorded by the profile_types_with_high_fidelity 
        opcode. Specifically: function parameters, function return values,
        function 'this' value, get_by_id, get_by_value, resolve nodes, function return 
        values at the call site. Added more flags to the profile_types_with_high_fidelity
        opcode so more focused tasks can take place when the instruction is
        being linked in CodeBlock. Re-worked the type profiler to search 
        through character offset ranges when asked for the type of an expression
        at a given offset. Removed redundant calls to Structure::toStructureShape
        in HighFidelityLog and TypeSet by caching calls based on StructureID.

        * bytecode/BytecodeList.json:
        * bytecode/BytecodeUseDef.h:
        (JSC::computeUsesForBytecodeOffset):
        (JSC::computeDefsForBytecodeOffset):
        * bytecode/CodeBlock.cpp:
        (JSC::CodeBlock::CodeBlock):
        (JSC::CodeBlock::finalizeUnconditionally):
        (JSC::CodeBlock::scopeDependentProfile):
        * bytecode/CodeBlock.h:
        (JSC::CodeBlock::returnStatementTypeSet):
        * bytecode/TypeLocation.h:
        * bytecode/UnlinkedCodeBlock.cpp:
        (JSC::UnlinkedCodeBlock::highFidelityTypeProfileExpressionInfoForBytecodeOffset):
        (JSC::UnlinkedCodeBlock::addHighFidelityTypeProfileExpressionInfo):
        * bytecode/UnlinkedCodeBlock.h:
        * bytecompiler/BytecodeGenerator.cpp:
        (JSC::BytecodeGenerator::emitMove):
        (JSC::BytecodeGenerator::emitProfileTypesWithHighFidelity):
        (JSC::BytecodeGenerator::emitGetFromScopeWithProfile):
        (JSC::BytecodeGenerator::emitPutToScope):
        (JSC::BytecodeGenerator::emitPutToScopeWithProfile):
        (JSC::BytecodeGenerator::emitPutById):
        (JSC::BytecodeGenerator::emitPutByVal):
        * bytecompiler/BytecodeGenerator.h:
        (JSC::BytecodeGenerator::emitHighFidelityTypeProfilingExpressionInfo):
        * bytecompiler/NodesCodegen.cpp:
        (JSC::ResolveNode::emitBytecode):
        (JSC::BracketAccessorNode::emitBytecode):
        (JSC::DotAccessorNode::emitBytecode):
        (JSC::FunctionCallValueNode::emitBytecode):
        (JSC::FunctionCallResolveNode::emitBytecode):
        (JSC::FunctionCallBracketNode::emitBytecode):
        (JSC::FunctionCallDotNode::emitBytecode):
        (JSC::CallFunctionCallDotNode::emitBytecode):
        (JSC::ApplyFunctionCallDotNode::emitBytecode):
        (JSC::PostfixNode::emitResolve):
        (JSC::PostfixNode::emitBracket):
        (JSC::PostfixNode::emitDot):
        (JSC::PrefixNode::emitResolve):
        (JSC::PrefixNode::emitBracket):
        (JSC::PrefixNode::emitDot):
        (JSC::ReadModifyResolveNode::emitBytecode):
        (JSC::AssignResolveNode::emitBytecode):
        (JSC::AssignDotNode::emitBytecode):
        (JSC::ReadModifyDotNode::emitBytecode):
        (JSC::AssignBracketNode::emitBytecode):
        (JSC::ReadModifyBracketNode::emitBytecode):
        (JSC::ReturnNode::emitBytecode):
        (JSC::FunctionBodyNode::emitBytecode):
        * inspector/agents/InspectorRuntimeAgent.cpp:
        (Inspector::InspectorRuntimeAgent::getRuntimeTypeForVariableAtOffset):
        (Inspector::InspectorRuntimeAgent::getRuntimeTypeForVariableInTextRange): Deleted.
        * inspector/agents/InspectorRuntimeAgent.h:
        * inspector/protocol/Runtime.json:
        * llint/LLIntSlowPaths.cpp:
        (JSC::LLInt::getFromScopeCommon):
        (JSC::LLInt::LLINT_SLOW_PATH_DECL):
        * llint/LLIntSlowPaths.h:
        * llint/LowLevelInterpreter.asm:
        * runtime/HighFidelityLog.cpp:
        (JSC::HighFidelityLog::processHighFidelityLog):
        (JSC::HighFidelityLog::actuallyProcessLogThreadFunction):
        (JSC::HighFidelityLog::recordTypeInformationForLocation): Deleted.
        * runtime/HighFidelityLog.h:
        (JSC::HighFidelityLog::recordTypeInformationForLocation):
        * runtime/HighFidelityTypeProfiler.cpp:
        (JSC::HighFidelityTypeProfiler::getTypesForVariableInAtOffset):
        (JSC::HighFidelityTypeProfiler::getGlobalTypesForVariableAtOffset):
        (JSC::HighFidelityTypeProfiler::getLocalTypesForVariableAtOffset):
        (JSC::HighFidelityTypeProfiler::insertNewLocation):
        (JSC::HighFidelityTypeProfiler::findLocation):
        (JSC::HighFidelityTypeProfiler::getTypesForVariableInRange): Deleted.
        (JSC::HighFidelityTypeProfiler::getGlobalTypesForVariableInRange): Deleted.
        (JSC::HighFidelityTypeProfiler::getLocalTypesForVariableInRange): Deleted.
        (JSC::HighFidelityTypeProfiler::getLocationBasedHash): Deleted.
        * runtime/HighFidelityTypeProfiler.h:
        (JSC::LocationKey::LocationKey): Deleted.
        (JSC::LocationKey::hash): Deleted.
        (JSC::LocationKey::operator==): Deleted.
        * runtime/Structure.h:
        * runtime/TypeSet.cpp:
        (JSC::TypeSet::TypeSet):
        (JSC::TypeSet::addTypeForValue):
        (JSC::TypeSet::seenTypes):
        (JSC::TypeSet::removeDuplicatesInStructureHistory): Deleted.
        * runtime/TypeSet.h:
        * runtime/VM.cpp:
        (JSC::VM::getTypesForVariableAtOffset):
        (JSC::VM::dumpHighFidelityProfilingTypes):
        (JSC::VM::getTypesForVariableInRange): Deleted.
        * runtime/VM.h:

[Filip Pizlo]

Comment on attachment 234237 [details]
patch

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

R=me, but can you post a new patch to a different bug, and also fix the one indentation issue?

> Source/JavaScriptCore/bytecode/UnlinkedCodeBlock.h:584
> +        public:

You don't need this.  Struct members are public by default.  Also, ordinarily this wouldn't be indented.  Example:

class Foo {
public:
    int m_bar;
}

Notice how "public:" aligns with "class".

> Source/JavaScriptCore/bytecode/UnlinkedCodeBlock.h:586
> +            unsigned m_startDivot;
> +            unsigned m_endDivot;

Deindent by four spaces.

[Mark Lam]

Landed missing build changes in r172184: <http://trac.webkit.org/r172184>.