Skip to content
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

Patch sets after setting up a clean history #13

Merged
merged 3 commits into from
May 18, 2019
Merged

Patch sets after setting up a clean history #13

merged 3 commits into from
May 18, 2019

Conversation

RAJAGOPALAN-GANGADHARAN

No description provided.

…BRoster

I guess the problem was it doesnt consider null as a good arguement just a marker so we need to pass i-1 as the arg count. This briefly
allows me to launch the webprocess app and i was able to see the debug messages on the terminal itself.
}
#endif

entry_ref processRef(BString path)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could be in ProcessLauncherHaiku, so you don't need to modify the .h file.

{
BEntry pathEntry(path);
if(!pathEntry.Exists())
ASSERT_NOT_REACHED();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indentation is incorrect. Also using assert is not very nice, could the function instead return an error code?

namespace WebKit {

SharedMemory::Handle::Handle()
{

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we disabled sockets we have to write this on our own

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, and we can use create_area and that family of functions to do it the native way

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh so no need to use Add data and use Add Pointers instead as i believe they use shared memory?

}
bool Connection::sendOutgoingMessage(std::unique_ptr<Encoder> encoder)
{
fprintf(stderr,"%s\n",__PRETTY_FUNCTION__);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A BMessage can be created and encoder can be attached to it by adding parameters like encoder.buffer and encoder.size onto bmessage where we can decode from recieveing end

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does Encoder encode the data to a flat buffer necessarily? I thought it would be able to natively fill a BMessage with the high level data (so that PrintToStream on the message does something meaningful, for example).

Well, either way works.

}
void Connection::runReadEventLoop()
{

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

probably start a loop here and attach handler to look for messages

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We already have a BApplication running, so it will be a little more complex I guess. Unless these things are run as separate threads?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm what if we just attach a handler to exisiting looper?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok sorry that doesnt make any sense 😅

}
bool Connection::sendOutgoingMessage(std::unique_ptr<Encoder> encoder)
{
BMessage processMessage('ipcm');

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one is incomplete the buffer is of type uint8_t* so i did add data im not sure if its right

@@ -39,7 +39,8 @@ void initializeAuxiliaryProcess<NetworkProcess>(AuxiliaryProcessInitializationPa

int NetworkProcessMainUnix(int argc, char** argv)
{
return AuxiliaryProcessMain<NetworkProcess, AuxiliaryProcessMainBase>(argc, argv);
//return AuxiliaryProcessMain<NetworkProcess, AuxiliaryProcessMainBase>(argc, argv);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for now focussing on webprocess so will probably create a template class for both webprocess and netwrokprocess BApplication

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be better if we could use the standard AuxiliaryProcessMain here. Especially as this now returns immediately, whereas before it was blocking the execution, right?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you explain this me again I couldnt understand this 😕

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think AuxiliaryProcessMain is like a "main" function, it does not return until the program is done, and when it returns, the program exits.

So if we do nothing here, the program exits immediately, right? And that's not what we want.

Well, let's work a bit on AuxiliaryProcess and get back to this.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh ok got it so we could Run our Application inside AuxProcessMain so it exists only after its done

Initial implementation of IPC for haiku will add it to description each time i amend
1)added empty functions
2)fixed building issues and successfully created a connection between webprocess
3)Added a message handler to look out for incoming messages to runloop
4) Created new aux support so we can attach all handlers to the main apps looper
@@ -143,6 +148,9 @@ class Connection : public ThreadSafeRefCounted<Connection, WTF::DestructionThrea
typedef HANDLE Identifier;
static bool createServerAndClientIdentifiers(Identifier& serverIdentifier, Identifier& clientIdentifier);
static bool identifierIsValid(Identifier identifier) { return !!identifier; }
#elif PLATFORM(HAIKU)
typedef team_id Identifier;
static bool identifierIsValid(Identifier identifier) { return BMessenger(NULL,identifier).IsValid(); }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't ther a cheaper way? For example can we ask BRoster if the team_id exists and is a BApplication?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

be_roster.GetRunningAppInfo sounds like it should work.

@@ -39,7 +39,8 @@ void initializeAuxiliaryProcess<NetworkProcess>(AuxiliaryProcessInitializationPa

int NetworkProcessMainUnix(int argc, char** argv)
{
return AuxiliaryProcessMain<NetworkProcess, AuxiliaryProcessMainBase>(argc, argv);
//return AuxiliaryProcessMain<NetworkProcess, AuxiliaryProcessMainBase>(argc, argv);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be better if we could use the standard AuxiliaryProcessMain here. Especially as this now returns immediately, whereas before it was blocking the execution, right?

@@ -398,6 +406,12 @@ class Connection : public ThreadSafeRefCounted<Connection, WTF::DestructionThrea
std::unique_ptr<Encoder> m_pendingWriteEncoder;
EventListener m_writeListener;
HANDLE m_connectionPipe { INVALID_HANDLE_VALUE };
#elif PLATFORM(HAIKU)
Identifier m_connectedProcess;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

beware of indentation

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Whats the identation to be followed this is not how how it looks in Pe it looks rightly idented but here it looks differently

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would recommend Koder instead of Pe then! I think it can show tabs and spaces.
Unfortunately the coding style is a mixup of both, so try to stick to whatever is used in the file you're editing, for now.


}
void Connection::runReadEventLoop()
{
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does this do on other platforms? Does it start a new thread there?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No they get attached to the work queues thread. In our case we attach to application loop

BString processIdentifier;
BString processIdentifier,processID;
team_id UIProcessID = getpid();
processID.SetToFormat("%ld",UIProcessID);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what's the difference between processID and processIdentifier? Can we use better naming?

@@ -77,15 +98,16 @@ void ProcessLauncher::launchProcess()
#endif
argv[i++] = executablePath.String();
argv[i++] = processIdentifier.String();
argv[i++] = processID.String();
// TODO pass our team_id so the web process can message us?
argv[i++] = nullptr;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can remove this and the i-1 below.

@@ -90,6 +90,8 @@ void WebInspector::setFrontendConnection(IPC::Attachment encodedConnectionIdenti
IPC::Connection::Identifier connectionIdentifier(encodedConnectionIdentifier.port());
#elif OS(WINDOWS)
IPC::Connection::Identifier connectionIdentifier(encodedConnectionIdentifier.handle());
#elif PLATFORM(HAIKU)
IPC::Connection::Identifier connectionIdentifier(NULL);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this can be properly implemented now?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah!

fprintf(stderr, "%s\n", __PRETTY_FUNCTION__);
RunLoop::initializeMainRunLoop();
RunLoop::run();
AuxiliaryProcessMain<WebProcess>(argc,argv);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems a bit backwards. I think AuxiliaryProcessMain should be in charge of creating a BApplication, if that's possible.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes AuxiliaryProcessMain creating will be much better as it will be easier to create a template BApllication so that we can use for both webprocess and network process

@pulkomandy pulkomandy merged commit 8cad8e3 into haiku:webkit2 May 18, 2019
@RAJAGOPALAN-GANGADHARAN RAJAGOPALAN-GANGADHARAN deleted the copywebkit2 branch June 29, 2019 19:13
pulkomandy pushed a commit that referenced this pull request Jan 12, 2020
…wind and lldb.

https://bugs.webkit.org/show_bug.cgi?id=205050

Reviewed by Michael Saboff.

Before this patch, the stack trace from inside a probe function is cut off at ctiMasmProbeTrampoline:

    (lldb) bt
    * thread #1, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0xbbadbeef)
        ...
        frame #4: 0x0000000100824607 JavaScriptCore`WTF::Function<void (JSC::Probe::Context&)>::operator(this=0x000000010b88bd00, in=0x00007ffeefbfd400)(JSC::Probe::Context&) const at Function.h:79:35
        frame #5: 0x0000000100823996 JavaScriptCore`JSC::stdFunctionCallback(context=0x00007ffeefbfd400) at MacroAssembler.cpp:53:5
        frame #6: 0x000000010082701e JavaScriptCore`JSC::Probe::executeProbe(state=0x00007ffeefbfd480) at ProbeContext.cpp:51:5
        frame #7: 0x000000010082614b JavaScriptCore`ctiMasmProbeTrampoline + 299
    (lldb) 

After this patch, we'll now get the full stack trace from inside the probe function:

    (lldb) bt
    * thread #1, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0xbbadbeef)
        ...
        frame #4: 0x0000000100826d17 JavaScriptCore`WTF::Function<void (JSC::Probe::Context&)>::operator(this=0x0000000106b878f8, in=0x00007ffeefbfd400)(JSC::Probe::Context&) const at Function.h:79:35
        frame #5: 0x0000000100826106 JavaScriptCore`JSC::stdFunctionCallback(context=0x00007ffeefbfd400) at MacroAssembler.cpp:53:5
        frame #6: 0x000000010082986e JavaScriptCore`JSC::Probe::executeProbe(state=0x00007ffeefbfd480) at ProbeContext.cpp:51:5
        frame #7: 0x00000001008289a2 JavaScriptCore`ctiMasmProbeTrampoline + 338
        frame #8: 0x0000466db28025be
        frame #9: 0x0000000100754ffc JavaScriptCore`llint_entry at LowLevelInterpreter.asm:994
        frame #10: 0x0000000100738173 JavaScriptCore`vmEntryToJavaScript at LowLevelInterpreter64.asm:307
        frame #11: 0x0000000101489307 JavaScriptCore`JSC::JITCode::execute(this=0x0000000106ba1520, vm=0x0000000106d00000, protoCallFrame=0x00007ffeefbfd9b8) at JITCodeInlines.h:38:38
        frame #12: 0x0000000101488982 JavaScriptCore`JSC::Interpreter::executeProgram(this=0x0000000106bfd1f8, source=0x00007ffeefbff090, (null)=0x000000010d0e0000, thisObj=0x000000010d0e8020) at Interpreter.cpp:847:51
        frame #13: 0x00000001017d1f9c JavaScriptCore`JSC::evaluate(globalObject=0x000000010d0e0000, source=0x00007ffeefbff090, thisValue=JSValue @ 0x00007ffeefbfef60, returnedException=0x00007ffeefbff0b0) at Completion.cpp:146:38
        frame #14: 0x000000010005838f jsc`runWithOptions(globalObject=0x000000010d0e0000, options=0x00007ffeefbff620, success=0x00007ffeefbff48b) at jsc.cpp:2670:35
        frame #15: 0x000000010002a0da jsc`jscmain(this=0x00007ffeefbff5a0, vm=0x0000000106d00000, globalObject=0x000000010d0e0000, success=0x00007ffeefbff48b)::$_6::operator()(JSC::VM&, GlobalObject*, bool&) const at jsc.cpp:3157:13
        frame #16: 0x0000000100006eff jsc`int runJSC<jscmain(int, char**)::$_6>(options=0x00007ffeefbff620, isWorker=false, func=0x00007ffeefbff5a0)::$_6 const&) at jsc.cpp:3003:9
        frame #17: 0x0000000100005988 jsc`jscmain(argc=10, argv=0x00007ffeefbff6c8) at jsc.cpp:3150:18
        frame #18: 0x000000010000575e jsc`main(argc=10, argv=0x00007ffeefbff6c8) at jsc.cpp:2498:15
        frame #19: 0x00007fff6cfc4da9 libdyld.dylib`start + 1
        frame #20: 0x00007fff6cfc4da9 libdyld.dylib`start + 1
    (lldb)

The difference is that the x86_64 ctiMasmProbeTrampoline now uses the standard
function prologue, and keeps %rbp pointing to trampoline function's semblance of
a frame that libunwind can understand while it calls the probe function.

* assembler/MacroAssemblerX86Common.cpp:



git-svn-id: http://svn.webkit.org/repository/webkit/trunk@253320 268f45cc-cd09-0410-ab3c-d52691b4dbfc
pulkomandy pushed a commit that referenced this pull request Feb 6, 2020
…rchability.

https://bugs.webkit.org/show_bug.cgi?id=207024

Reviewed by Saam Barati.

This patch applies the following changes:

1. Prefix Air and B2 dumps with a tierName prefix.
   The tierName prefix strings are as follows:

       "FTL ", "DFG ", "b3  ", "Air ", "asm "

   The choice to use a lowercase "b3" and "asm" with upper case "Air" is
   deliberate because I found this combination to be easier to read and scan as
   prefixes of the dump lines.  See dump samples below.

2. Make DFG node IDs consistently expressed as D@<node index> e.g. D@104.
   The definition of the node will be the id followed by a colon e.g. D@104:
   This makes it easy to search references to this node anywhere in the dump.

   Make B3 nodes expressed as b@<node index> e.g. b@542.
   This also makes it searchable since there's now no ambiguity between b@542 and
   D@542.

   The choice to use a lowercase "b" and an uppercase "D" is intentional because
   "b@542" and "d@542" looks too similar, and I prefer to not use too much
   uppercase.  Plus this makes the node consistent in capitalization with the
   tierName prefixes above of "b3  " and "DFG " respectively.

Here's a sample of what the dumps now look like:

DFG graph dump:
<code>
    ...
         6 55:   <-- foo#DFndCW:<0x62d0000b8140, bc#65, Call, known callee: Object: 0x62d000035920 with butterfly 0x0 (Structure %AN:Function), StructureID: 12711, numArgs+this = 1, numFixup = 0, stackOffset = -16 (loc0 maps to loc16)>
      3  6 55:   D@79:< 3:->    ArithAdd(Int32:Kill:D@95, Int32:D@42, Int32|PureNum|UseAsOther, Int32, CheckOverflow, Exits, bc#71, ExitValid)
      4  6 55:    D@3:<!0:->    KillStack(MustGen, loc7, W:Stack(loc7), ClobbersExit, bc#71, ExitInvalid)
      5  6 55:   D@85:<!0:->    MovHint(Check:Untyped:D@79, MustGen, loc7, W:SideState, ClobbersExit, bc#71, ExitInvalid)
      6  6 55:  D@102:< 1:->    CompareLess(Int32:D@79, Int32:D@89, Boolean|UseAsOther, Bool, Exits, bc#74, ExitValid)
      7  6 55:  D@104:<!0:->    Branch(KnownBoolean:Kill:D@102, MustGen, T:#1/w:10.000000, F:#7/w:1.000000, W:SideState, bc#74, ExitInvalid)
    ...
</code>

B3 graph dump:
<code>
    ...
    b3  BB#14: ; frequency = 10.000000
    b3    Predecessors: #13
    b3      Int32 b@531 = CheckAdd(b@10:WarmAny, $1(b@1):WarmAny, b@64:ColdAny, b@10:ColdAny, generator = 0x606000022e80, earlyClobbered = [], lateClobbered = [], usedRegisters = [], ExitsSideways|Reads:Top, D@79)
    b3      Int32 b@539 = LessThan(b@531, $100(b@578), D@102)
    b3      Void b@542 = Branch(b@539, Terminal, D@104)
    b3    Successors: Then:#2, Else:#15
    ...
</code>

Air graph dump:
<code>
    ...
    Air BB#5: ; frequency = 10.000000
    Air   Predecessors: #4
    Air     Move -96(%rbp), %rax, b@531
    Air     Patch &BranchAdd32(3,ForceLateUseUnlessRecoverable)3, Overflow, $1, %rax, -104(%rbp), -96(%rbp), b@531
    Air     Branch32 LessThan, %rax, $100, b@542
    Air   Successors: #1, #6
    ...
</code>

FTL disassembly dump:
<code>
    ...
    Air BB#5: ; frequency = 10.000000
    Air   Predecessors: #4
    DFG       D@42:< 2:->   JSConstant(JS|PureInt, Int32, Int32: 1, bc#0, ExitInvalid)
    DFG       D@79:< 3:->   ArithAdd(Int32:Kill:D@95, Int32:D@42, Int32|PureNum|UseAsOther, Int32, CheckOverflow, Exits, bc#71, ExitValid)
    b3            Int32 b@1 = Const32(1)
    b3            Int32 b@531 = CheckAdd(b@10:WarmAny, $1(b@1):WarmAny, b@64:ColdAny, b@10:ColdAny, generator = 0x606000022e80, earlyClobbered = [], lateClobbered = [], usedRegisters = [%rax, %rbx, %rbp, %r12], ExitsSideways|Reads:Top, D@79)
    Air               Move -96(%rbp), %rax, b@531
    asm                   0x4576b9c04712: mov -0x60(%rbp), %rax
    Air               Patch &BranchAdd32(3,ForceLateUseUnlessRecoverable)3, Overflow, $1, %rax, -104(%rbp), -96(%rbp), b@531
    asm                   0x4576b9c04716: inc %eax
    asm                   0x4576b9c04718: jo 0x4576b9c04861
    DFG       D@89:< 1:->   JSConstant(JS|PureNum|UseAsOther, NonBoolInt32, Int32: 100, bc#0, ExitInvalid)
    DFG      D@102:< 1:->   CompareLess(Int32:D@79, Int32:D@89, Boolean|UseAsOther, Bool, Exits, bc#74, ExitValid)
    DFG      D@104:<!0:->   Branch(KnownBoolean:Kill:D@102, MustGen, T:#1/w:10.000000, F:#7/w:1.000000, W:SideState, bc#74, ExitInvalid)
    b3            Int32 b@578 = Const32(100, D@89)
    b3            Int32 b@539 = LessThan(b@531, $100(b@578), D@102)
    b3            Void b@542 = Branch(b@539, Terminal, D@104)
    Air               Branch32 LessThan, %rax, $100, b@542
    asm                   0x4576b9c0471e: cmp $0x64, %eax
    asm                   0x4576b9c04721: jl 0x4576b9c0462f
    Air   Successors: #1, #6
    ...
</code>

* b3/B3BasicBlock.cpp:
(JSC::B3::BasicBlock::deepDump const):
* b3/B3Common.cpp:
* b3/B3Common.h:
* b3/B3Generate.cpp:
(JSC::B3::generateToAir):
* b3/B3Procedure.cpp:
(JSC::B3::Procedure::dump const):
* b3/B3Value.cpp:
* b3/air/AirBasicBlock.cpp:
(JSC::B3::Air::BasicBlock::deepDump const):
(JSC::B3::Air::BasicBlock::dumpHeader const):
(JSC::B3::Air::BasicBlock::dumpFooter const):
* b3/air/AirCode.cpp:
(JSC::B3::Air::Code::dump const):
* b3/air/AirCode.h:
* b3/air/AirDisassembler.cpp:
(JSC::B3::Air::Disassembler::dump):
* b3/air/AirGenerate.cpp:
(JSC::B3::Air::prepareForGeneration):
* dfg/DFGCommon.cpp:
* dfg/DFGCommon.h:
* dfg/DFGGraph.cpp:
(JSC::DFG::Graph::dump):
(JSC::DFG::Graph::dumpBlockHeader):
* dfg/DFGNode.cpp:
(WTF::printInternal):
* ftl/FTLCompile.cpp:
(JSC::FTL::compile):
* ftl/FTLCompile.h:
* ftl/FTLState.cpp:
(JSC::FTL::State::State):



git-svn-id: http://svn.webkit.org/repository/webkit/trunk@255482 268f45cc-cd09-0410-ab3c-d52691b4dbfc
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants