[Lldb-commits] [PATCH] D131159: [lldb] Use WSAEventSelect for MainLoop polling on windows

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In D131159#3743497 , @mstorsjo wrote:

> In D131159#3742370 , @labath wrote:
>
>> In D131159#3742364 , @mgorny wrote:
>>
>>> In D131159#3742235 , @mstorsjo 
>>> wrote:
>>>
 (They're only used in asserts.) I guess we should add void casts to mark 
 the variables as used outside of asserts.
>>>
>>> Actually, there's a dedicated `UNUSED_IF_ASSERT_DISABLED` macro for that. 
>>> Please use it instead.
>>
>> One of these days, I'm going to have to delete it. If void casts are good 
>> enough for the rest of llvm, I don't see why should we be any different.
>
> Ok, so is this discussion settled and I can go ahead and use void casts for 
> these trivial fixups? (I usually just push such patches directly without 
> passing via review.)

I'm might not call it "settled", but I don't think anybody will be too upset 
regardless of that you do. You won't be the first or the last one to push such 
patches, and that's the reason why I think that the UNUSED_IF_ASSERT_DISABLED 
macro is doomed to fail (or at least be used inconsistently).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D131159/new/

https://reviews.llvm.org/D131159

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D131159: [lldb] Use WSAEventSelect for MainLoop polling on windows

[Martin Storsjö via Phabricator via lldb-commits]

mstorsjo added a comment.

In D131159#3742370 , @labath wrote:

> In D131159#3742364 , @mgorny wrote:
>
>> In D131159#3742235 , @mstorsjo 
>> wrote:
>>
>>> (They're only used in asserts.) I guess we should add void casts to mark 
>>> the variables as used outside of asserts.
>>
>> Actually, there's a dedicated `UNUSED_IF_ASSERT_DISABLED` macro for that. 
>> Please use it instead.
>
> One of these days, I'm going to have to delete it. If void casts are good 
> enough for the rest of llvm, I don't see why should we be any different.

Ok, so is this discussion settled and I can go ahead and use void casts for 
these trivial fixups? (I usually just push such patches directly without 
passing via review.)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D131159/new/

https://reviews.llvm.org/D131159

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D131159: [lldb] Use WSAEventSelect for MainLoop polling on windows

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In D131159#3742364 , @mgorny wrote:

> In D131159#3742235 , @mstorsjo 
> wrote:
>
>> (They're only used in asserts.) I guess we should add void casts to mark the 
>> variables as used outside of asserts.
>
> Actually, there's a dedicated `UNUSED_IF_ASSERT_DISABLED` macro for that. 
> Please use it instead.

One of these days, I'm going to have to delete it. If void casts are good 
enough for the rest of llvm, I don't see why should we be any different.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D131159/new/

https://reviews.llvm.org/D131159

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D131159: [lldb] Use WSAEventSelect for MainLoop polling on windows

[Michał Górny via Phabricator via lldb-commits]

mgorny added a comment.

In D131159#3742235 , @mstorsjo wrote:

> (They're only used in asserts.) I guess we should add void casts to mark the 
> variables as used outside of asserts.

Actually, there's a dedicated `UNUSED_IF_ASSERT_DISABLED` macro for that. 
Please use it instead.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D131159/new/

https://reviews.llvm.org/D131159

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D131159: [lldb] Use WSAEventSelect for MainLoop polling on windows

[Martin Storsjö via Phabricator via lldb-commits]

mstorsjo added a comment.

FYI, this patch added some new compilation warnings:

  [4055/7050] Building CXX object 
tools/lldb/source/Host/CMakeFiles/lldbHost.dir/windows/MainLoopWindows.cpp.obj
  llvm-project/lldb/source/Host/windows/MainLoopWindows.cpp:28:9: warning: 
unused variable 'result' [-Wunused-variable]
  int result = WSAEventSelect(fd, info.event, FD_READ | FD_ACCEPT | 
FD_CLOSE);
  ^
  llvm-project/lldb/source/Host/windows/MainLoopWindows.cpp:38:9: warning: 
unused variable 'result' [-Wunused-variable]
  int result = WSAEventSelect(fd.first, WSA_INVALID_EVENT, 0);
  ^
  llvm-project/lldb/source/Host/windows/MainLoopWindows.cpp:86:8: warning: 
unused variable 'result' [-Wunused-variable]
BOOL result = WSACloseEvent(it->second.event);
 ^
  3 warnings generated.

(They're only used in asserts.) I guess we should add void casts to mark the 
variables as used outside of asserts.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D131159/new/

https://reviews.llvm.org/D131159

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D131159: [lldb] Use WSAEventSelect for MainLoop polling on windows

[Pavel Labath via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGc74c17f37aa9: [lldb] Use WSAEventSelect for MainLoop polling 
on windows (authored by labath).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D131159/new/

https://reviews.llvm.org/D131159

Files:
  lldb/include/lldb/Host/MainLoop.h
  lldb/include/lldb/Host/MainLoopBase.h
  lldb/include/lldb/Host/posix/MainLoopPosix.h
  lldb/include/lldb/Host/windows/MainLoopWindows.h
  lldb/source/Host/CMakeLists.txt
  lldb/source/Host/common/MainLoop.cpp
  lldb/source/Host/common/MainLoopBase.cpp
  lldb/source/Host/posix/MainLoopPosix.cpp
  lldb/source/Host/windows/MainLoopWindows.cpp

Index: lldb/source/Host/windows/MainLoopWindows.cpp
===
--- /dev/null
+++ lldb/source/Host/windows/MainLoopWindows.cpp
@@ -0,0 +1,115 @@
+//===-- MainLoopWindows.cpp ---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "lldb/Host/windows/MainLoopWindows.h"
+#include "lldb/Host/Config.h"
+#include "lldb/Utility/Status.h"
+#include "llvm/Config/llvm-config.h"
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+using namespace lldb;
+using namespace lldb_private;
+
+llvm::Expected MainLoopWindows::Poll() {
+  std::vector read_events;
+  read_events.reserve(m_read_fds.size());
+  for (auto &[fd, info] : m_read_fds) {
+int result = WSAEventSelect(fd, info.event, FD_READ | FD_ACCEPT | FD_CLOSE);
+assert(result == 0);
+
+read_events.push_back(info.event);
+  }
+
+  DWORD result = WSAWaitForMultipleEvents(
+  read_events.size(), read_events.data(), FALSE, WSA_INFINITE, FALSE);
+
+  for (auto  : m_read_fds) {
+int result = WSAEventSelect(fd.first, WSA_INVALID_EVENT, 0);
+assert(result == 0);
+  }
+
+  if (result >= WSA_WAIT_EVENT_0 &&
+  result < WSA_WAIT_EVENT_0 + read_events.size())
+return result - WSA_WAIT_EVENT_0;
+
+  return llvm::createStringError(llvm::inconvertibleErrorCode(),
+ "WSAWaitForMultipleEvents failed");
+}
+
+MainLoopWindows::ReadHandleUP
+MainLoopWindows::RegisterReadObject(const IOObjectSP _sp,
+const Callback , Status ) {
+  if (!object_sp || !object_sp->IsValid()) {
+error.SetErrorString("IO object is not valid.");
+return nullptr;
+  }
+  if (object_sp->GetFdType() != IOObject::eFDTypeSocket) {
+error.SetErrorString(
+"MainLoopWindows: non-socket types unsupported on Windows");
+return nullptr;
+  }
+
+  WSAEVENT event = WSACreateEvent();
+  if (event == WSA_INVALID_EVENT) {
+error.SetErrorStringWithFormat("Cannot create monitoring event.");
+return nullptr;
+  }
+
+  const bool inserted =
+  m_read_fds
+  .try_emplace(object_sp->GetWaitableHandle(), FdInfo{event, callback})
+  .second;
+  if (!inserted) {
+WSACloseEvent(event);
+error.SetErrorStringWithFormat("File descriptor %d already monitored.",
+   object_sp->GetWaitableHandle());
+return nullptr;
+  }
+
+  return CreateReadHandle(object_sp);
+}
+
+void MainLoopWindows::UnregisterReadObject(IOObject::WaitableHandle handle) {
+  auto it = m_read_fds.find(handle);
+  assert(it != m_read_fds.end());
+  BOOL result = WSACloseEvent(it->second.event);
+  assert(result == TRUE);
+  m_read_fds.erase(it);
+}
+
+void MainLoopWindows::ProcessReadObject(IOObject::WaitableHandle handle) {
+  auto it = m_read_fds.find(handle);
+  if (it != m_read_fds.end())
+it->second.callback(*this); // Do the work
+}
+
+Status MainLoopWindows::Run() {
+  m_terminate_request = false;
+
+  Status error;
+
+  // run until termination or until we run out of things to listen to
+  while (!m_terminate_request && !m_read_fds.empty()) {
+
+llvm::Expected signaled_event = Poll();
+if (!signaled_event)
+  return Status(signaled_event.takeError());
+
+auto  = *std::next(m_read_fds.begin(), *signaled_event);
+
+ProcessReadObject(KV.first);
+ProcessPendingCallbacks();
+  }
+  return Status();
+}
Index: lldb/source/Host/posix/MainLoopPosix.cpp
===
--- lldb/source/Host/posix/MainLoopPosix.cpp
+++ lldb/source/Host/posix/MainLoopPosix.cpp
@@ -1,4 +1,4 @@
-//===-- MainLoop.cpp --===//
+//===-- MainLoopPosix.cpp -===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
@@ -6,12 +6,11 @@
 //
 

[Lldb-commits] [PATCH] D131159: [lldb] Use WSAEventSelect for MainLoop polling on windows

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

Thank you, Martin and Alvin for trying this out. Setting `LLDB_USE_LLDB_SERVER` 
definitely increases the number of tests that use lldb-server, but I wouldn't 
really say it's necessary, as I would expect that a change like this either 
works, or fails spectacularly.

Can I assume that you are ok with checking this in?

In D131159#3709965 , @labath wrote:

> As for performance, it's definitely slower than not creating the events every 
> time around, but I doubt that difference would be noticeable. However, now 
> that I think about it, I think the repeating the call to `WSAEventSelect` 
> should be sufficient to get the level-triggered behavior, even if it's just 
> reusing the same event object. Let me try how that works.

I have implemented this now.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D131159/new/

https://reviews.llvm.org/D131159

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D131159: [lldb] Use WSAEventSelect for MainLoop polling on windows

[Pavel Labath via Phabricator via lldb-commits]

labath updated this revision to Diff 453686.
labath added a comment.

Switch to the implementation which uses persistent WSAEvents for
listening. I have no idea whether this makes a difference in practice,
but it /feels/ more efficient.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D131159/new/

https://reviews.llvm.org/D131159

Files:
  lldb/include/lldb/Host/MainLoop.h
  lldb/include/lldb/Host/MainLoopBase.h
  lldb/include/lldb/Host/posix/MainLoopPosix.h
  lldb/include/lldb/Host/windows/MainLoopWindows.h
  lldb/source/Host/CMakeLists.txt
  lldb/source/Host/common/MainLoop.cpp
  lldb/source/Host/common/MainLoopBase.cpp
  lldb/source/Host/posix/MainLoopPosix.cpp
  lldb/source/Host/windows/MainLoopWindows.cpp

Index: lldb/source/Host/windows/MainLoopWindows.cpp
===
--- /dev/null
+++ lldb/source/Host/windows/MainLoopWindows.cpp
@@ -0,0 +1,115 @@
+//===-- MainLoopWindows.cpp ---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "lldb/Host/windows/MainLoopWindows.h"
+#include "lldb/Host/Config.h"
+#include "lldb/Utility/Status.h"
+#include "llvm/Config/llvm-config.h"
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+using namespace lldb;
+using namespace lldb_private;
+
+llvm::Expected MainLoopWindows::Poll() {
+  std::vector read_events;
+  read_events.reserve(m_read_fds.size());
+  for (auto &[fd, info] : m_read_fds) {
+int result = WSAEventSelect(fd, info.event, FD_READ | FD_ACCEPT | FD_CLOSE);
+assert(result == 0);
+
+read_events.push_back(info.event);
+  }
+
+  DWORD result = WSAWaitForMultipleEvents(
+  read_events.size(), read_events.data(), FALSE, WSA_INFINITE, FALSE);
+
+  for (auto  : m_read_fds) {
+int result = WSAEventSelect(fd.first, WSA_INVALID_EVENT, 0);
+assert(result == 0);
+  }
+
+  if (result >= WSA_WAIT_EVENT_0 &&
+  result < WSA_WAIT_EVENT_0 + read_events.size())
+return result - WSA_WAIT_EVENT_0;
+
+  return llvm::createStringError(llvm::inconvertibleErrorCode(),
+ "WSAWaitForMultipleEvents failed");
+}
+
+MainLoopWindows::ReadHandleUP
+MainLoopWindows::RegisterReadObject(const IOObjectSP _sp,
+const Callback , Status ) {
+  if (!object_sp || !object_sp->IsValid()) {
+error.SetErrorString("IO object is not valid.");
+return nullptr;
+  }
+  if (object_sp->GetFdType() != IOObject::eFDTypeSocket) {
+error.SetErrorString(
+"MainLoopWindows: non-socket types unsupported on Windows");
+return nullptr;
+  }
+
+  WSAEVENT event = WSACreateEvent();
+  if (event == WSA_INVALID_EVENT) {
+error.SetErrorStringWithFormat("Cannot create monitoring event.");
+return nullptr;
+  }
+
+  const bool inserted =
+  m_read_fds
+  .try_emplace(object_sp->GetWaitableHandle(), FdInfo{event, callback})
+  .second;
+  if (!inserted) {
+WSACloseEvent(event);
+error.SetErrorStringWithFormat("File descriptor %d already monitored.",
+   object_sp->GetWaitableHandle());
+return nullptr;
+  }
+
+  return CreateReadHandle(object_sp);
+}
+
+void MainLoopWindows::UnregisterReadObject(IOObject::WaitableHandle handle) {
+  auto it = m_read_fds.find(handle);
+  assert(it != m_read_fds.end());
+  BOOL result = WSACloseEvent(it->second.event);
+  assert(result == TRUE);
+  m_read_fds.erase(it);
+}
+
+void MainLoopWindows::ProcessReadObject(IOObject::WaitableHandle handle) {
+  auto it = m_read_fds.find(handle);
+  if (it != m_read_fds.end())
+it->second.callback(*this); // Do the work
+}
+
+Status MainLoopWindows::Run() {
+  m_terminate_request = false;
+
+  Status error;
+
+  // run until termination or until we run out of things to listen to
+  while (!m_terminate_request && !m_read_fds.empty()) {
+
+llvm::Expected signaled_event = Poll();
+if (!signaled_event)
+  return Status(signaled_event.takeError());
+
+auto  = *std::next(m_read_fds.begin(), *signaled_event);
+
+ProcessReadObject(KV.first);
+ProcessPendingCallbacks();
+  }
+  return Status();
+}
Index: lldb/source/Host/posix/MainLoopPosix.cpp
===
--- lldb/source/Host/posix/MainLoopPosix.cpp
+++ lldb/source/Host/posix/MainLoopPosix.cpp
@@ -1,4 +1,4 @@
-//===-- MainLoop.cpp --===//
+//===-- MainLoopPosix.cpp -===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license 

[Lldb-commits] [PATCH] D131159: [lldb] Use WSAEventSelect for MainLoop polling on windows

[Alvin Wong via Phabricator via lldb-commits]

alvinhochun added a comment.

I gave this a spin with `LLDB_USE_LLDB_SERVER=1`, but there looks to be some 
pre-existing issues with the lldb-server implementation so I can't really test 
much normally using builds of Krita:

- Manually set breakpoints don't work (they stay unresolved?)
- Debugger apparently breaks on every first chance exceptions (running ASAN 
builds stops too often)
- Backtraces have no symbol names and seems to show bogus addresses

Other than that, it runs the debuggee and I can continue on exception stops. 
That's as much as I can tell.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D131159/new/

https://reviews.llvm.org/D131159

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D131159: [lldb] Use WSAEventSelect for MainLoop polling on windows

[Martin Storsjö via Phabricator via lldb-commits]

mstorsjo added a comment.

In D131159#3711024 , @mstorsjo wrote:

> In D131159#3709965 , @labath wrote:
>
>>> Can @alvinhochun take the patch for a spin? I think you've got more 
>>> realistic usecases of lldb to try it out in. (Do you want me to make a 
>>> build with the patch for you to try out?) @labath What's the best way to 
>>> exercise this code in actual use of lldb?
>>
>> I would appreciate that. I have tried to build it and run the test suite, 
>> but I didn't have a clean baseline, so I can't really say whether it works 
>> for everything. The main user of this code is the gdb-remote communication 
>> code, so debugging something via lldb-server (or running the lldb-server 
>> test suite) should be a good indicator of whether it works.
>
> I presume this is covered by running `ninja check-lldb`, or should I flip the 
> switch to prefer lldb-server on Windows before doing that? (I guess I can do 
> both.) I've got an environment where `check-lldb` mostly passes, I can check 
> that it doesn't regress it.

I tested out this patch with running `ninja check-lldb`. Out of the box, this 
patch doesn't regress anything - the tests still pass. (In my environment, 
there's a bunch of occasional stray failures already before, but by rerunning 
the tests a couple times, I can get a clean run.)

If I edit `ShouldUseLLDBServer()` in 
lldb/source/Plugins/Process/Windows/Common/ProcessWindows.cpp to always return 
true (before applying this patch), most tests still pass, but I have 11 tests 
that hang so that I end up having to kill lldb-server. With this patch on top, 
I still get the same set of 11 hanging tests, so I think that means that this 
patch doesn't regress anything with respect to that.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D131159/new/

https://reviews.llvm.org/D131159

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D131159: [lldb] Use WSAEventSelect for MainLoop polling on windows

[Martin Storsjö via Phabricator via lldb-commits]

mstorsjo added a comment.

In D131159#3709965 , @labath wrote:

>> Can @alvinhochun take the patch for a spin? I think you've got more 
>> realistic usecases of lldb to try it out in. (Do you want me to make a build 
>> with the patch for you to try out?) @labath What's the best way to exercise 
>> this code in actual use of lldb?
>
> I would appreciate that. I have tried to build it and run the test suite, but 
> I didn't have a clean baseline, so I can't really say whether it works for 
> everything. The main user of this code is the gdb-remote communication code, 
> so debugging something via lldb-server (or running the lldb-server test 
> suite) should be a good indicator of whether it works.

I presume this is covered by running `ninja check-lldb`, or should I flip the 
switch to prefer lldb-server on Windows before doing that? (I guess I can do 
both.) I've got an environment where `check-lldb` mostly passes, I can check 
that it doesn't regress it.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D131159/new/

https://reviews.llvm.org/D131159

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D131159: [lldb] Use WSAEventSelect for MainLoop polling on windows

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In D131159#3707741 , @mstorsjo wrote:

> I don't really have experience with this API - is this pattern (creating and 
> closing event objects for each poll run) the common way of using this API? Is 
> there any potential performance risk compared with keeping event objects 
> around (if that's possible?).

Well.. I can't exactly say I'm familiar with the APIs either, but I suspect 
this is not the "common" way of using them. It is, however, the closest I could 
get to the purely level-triggered behavior that one gets with select/poll on 
unix.  Normally (with event reuse), windows requires that one calls the "reset" 
function (e.g. `recv`) at least once (the call itself is sufficient, it does 
not have to actually read all pending data) before the event can be triggered 
again. I think possible that nothing depends on this behavior (or could be 
fixed to not depend on it) -- after all you're usually doing this because you 
actually want to read from the socket. However, we did have one unit test which 
relied on this. That was mostly because it was being lazy (testing other 
things), but I wanted to start out with something safe-ish.

As for performance, it's definitely slower than not creating the events every 
time around, but I doubt that difference would be noticeable. However, now that 
I think about it, I think the repeating the call to `WSAEventSelect` should be 
sufficient to get the level-triggered behavior, even if it's just reusing the 
same event object. Let me try how that works.

> Can @alvinhochun take the patch for a spin? I think you've got more realistic 
> usecases of lldb to try it out in. (Do you want me to make a build with the 
> patch for you to try out?) @labath What's the best way to exercise this code 
> in actual use of lldb?

I would appreciate that. I have tried to build it and run the test suite, but I 
didn't have a clean baseline, so I can't really say whether it works for 
everything. The main user of this code is the gdb-remote communication code, so 
debugging something via lldb-server (or running the lldb-server test suite) 
should be a good indicator of whether it works.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D131159/new/

https://reviews.llvm.org/D131159

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D131159: [lldb] Use WSAEventSelect for MainLoop polling on windows

[Martin Storsjö via Phabricator via lldb-commits]

mstorsjo added a subscriber: alvinhochun.
mstorsjo added a comment.

I don't really have experience with this API - is this pattern (creating and 
closing event objects for each poll run) the common way of using this API? Is 
there any potential performance risk compared with keeping event objects around 
(if that's possible?).

Can @alvinhochun take the patch for a spin? I think you've got more realistic 
usecases of lldb to try it out in. (Do you want me to make a build with the 
patch for you to try out?) @labath What's the best way to exercise this code in 
actual use of lldb?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D131159/new/

https://reviews.llvm.org/D131159

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D131159: [lldb] Use WSAEventSelect for MainLoop polling on windows

[Michał Górny via Phabricator via lldb-commits]

mgorny accepted this revision.
mgorny added a comment.

Thanks for doing this. The common and POSIX parts LGTM. Just one minor 
suggestion since you've touched that line.




Comment at: lldb/source/Host/posix/MainLoopPosix.cpp:124
   assert(ret == 0);
-  (void) ret;
+  (void)ret;
 




Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D131159/new/

https://reviews.llvm.org/D131159

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits