[Lldb-commits] [lldb] r223222 - Fix a hang on OSX while executing -exec-run.
Abid, Hafiz
Hafiz_Abid at mentor.com
Thu Dec 11 01:11:02 PST 2014
I will more thoroughly test all different combination on Linux to see how they affect performance. I will let you know what I find although it may be a few days. Please also check your patch on OSX to see it does not create any other issue before sending.
Thanks,
Abid
From: Ilia K [mailto:ki.stfu at gmail.com]
Sent: 10 December 2014 09:29
To: Abid, Hafiz
Cc: lldb-commits at cs.uiuc.edu
Subject: Re: [Lldb-commits] [lldb] r223222 - Fix a hang on OSX while executing -exec-run.
Hello Abid,
Thanks again for the fix :)
As far as I know this needed only on OSX and it isn't required on other platforms. But after it was committed 'lldb-mi --interpreter' began to load cpu over 100%!! It's very bad on my side. And as I understood it happened on Linux too. Of course it helped with hangs on OSX, but it made worse for Linux (and OSX). You said that 'select' patch increases time to run test many times, but I didn't face with it on OSX (although I think 'select' shouldn't affect it). I believe that it is necessary to revert 'ioctl' patch (to avoid overheads of CPU) and apply 'select' patch to OSX only (by using #ifdef __APPLE__ or by creating separate file).
I can create patch for it if you agree.
Thanks,
Ilia
On Thu, Dec 4, 2014 at 2:04 PM, Abid, Hafiz <Hafiz_Abid at mentor.com<mailto:Hafiz_Abid at mentor.com>> wrote:
Please send them. I will try.
Thanks,
Abid
From: Ilia K [mailto:ki.stfu at gmail.com<mailto:ki.stfu at gmail.com>]
Sent: 04 December 2014 11:02
To: Abid, Hafiz
Cc: lldb-commits at cs.uiuc.edu<mailto:lldb-commits at cs.uiuc.edu>
Subject: Re: [Lldb-commits] [lldb] r223222 - Fix a hang on OSX while executing -exec-run.
Abid,
Thank you.
I have a few more patches and possible I'll send some of them today. Could you review them too?
Thanks,
Ilia
On Thu, Dec 4, 2014 at 1:53 PM, Abid, Hafiz <Hafiz_Abid at mentor.com<mailto:Hafiz_Abid at mentor.com>> wrote:
I tried to remove setbuf() yesterday and it didn't affect, so I think it is not needed. Could you remove code related to setbuf() from my patch yourself?
Ok I will remove and then commit the patch. If you intend to contribute more patches. I would be good to get commit access.
Thanks,
Abid
From: Ilia K [mailto:ki.stfu at gmail.com<mailto:ki.stfu at gmail.com>]
Sent: 04 December 2014 10:41
To: Abid, Hafiz
Cc: lldb-commits at cs.uiuc.edu<mailto:lldb-commits at cs.uiuc.edu>
Subject: Re: [Lldb-commits] [lldb] r223222 - Fix a hang on OSX while executing -exec-run.
Hi Abid,
> It is now returning false when ioctl fails
I think that it's right, because status is unavailable.
> Regarding your question about setbuf, I thought that setting it to null makes ioctl type function to return when some input is available and not wait for the new line character
I realized that setbuf is not needed. As you said, it makes ioctl type function to return when some input is available and not wait for the new line character, but in canonical mode, you don't get control back until the new line character had typed.
> Can you try on OSX after removing the setbuf call
I tried to remove setbuf() yesterday and it didn't affect, so I think it is not needed. Could you remove code related to setbuf() from my patch yourself?
Thanks,
Ilia
On Thu, Dec 4, 2014 at 1:17 PM, Abid, Hafiz <Hafiz_Abid at mentor.com<mailto:Hafiz_Abid at mentor.com>> wrote:
Hi Ilia,
Thanks for the patch. It looks much cleaner. I was also not sure how tcgetattr type functions were helping with the original fix. Regarding your patch, I only have one comment. It is now returning false when ioctl fails. It can result in lldb-mi quitting. Previously this function was always returning true. I don’t think this is a big deal though.
Regarding your question about setbuf, I thought that setting it to null makes ioctl type function to return when some input is available and not wait for the new line character. I may be wrong here too. Can you try on OSX after removing the setbuf call. Does that change the behaviour in any way. Because I cannot see any noticeable difference on Linux.
Regards,
Abid
From: Ilia K [mailto:ki.stfu at gmail.com<mailto:ki.stfu at gmail.com>]
Sent: 03 December 2014 16:17
To: Abid, Hafiz
Cc: lldb-commits at cs.uiuc.edu<mailto:lldb-commits at cs.uiuc.edu>
Subject: Re: [Lldb-commits] [lldb] r223222 - Fix a hang on OSX while executing -exec-run.
Hi Abid,
Sorry for remarks, but there is a one problem with "ioctl" patch: it turns off canonical mode so we can't modify commands on input line correctly :(.
Also I added error checks and moved "setbuf(stdin, NULL)" into Initialize function to avoid ugly in-place initialization using static variable (but, if be honest I don't know why we need to disable bufferization before "ioctl" call).
I prepared new patch. Review it please.
Thanks,
Ilia
On Wed, Dec 3, 2014 at 3:52 PM, Ilia K <ki.stfu at gmail.com<mailto:ki.stfu at gmail.com>> wrote:
Thank you
On 3 Dec 2014 15:50, "Abid, Hafiz" <Hafiz_Abid at mentor.com<mailto:Hafiz_Abid at mentor.com>> wrote:
Thanks for catching this. I have removed the extra changes in r223227.
Thanks,
Abid
From: Ilia K [mailto:ki.stfu at gmail.com<mailto:ki.stfu at gmail.com>]
Sent: 03 December 2014 12:40
To: Abid, Hafiz
Cc: lldb-commits at cs.uiuc.edu<mailto:lldb-commits at cs.uiuc.edu>
Subject: RE: [Lldb-commits] [lldb] r223222 - Fix a hang on OSX while executing -exec-run.
Yes. "select" patch requires extra handling on exit.
Thanks,
Ilia
On 3 Dec 2014 15:37, "Abid, Hafiz" <Hafiz_Abid at mentor.com<mailto:Hafiz_Abid at mentor.com>> wrote:
You mean that just the change in ‘InputAvailable’ was needed? Rest of the changes were required with ‘select’.
Regards,
Abid
From: Ilia K [mailto:ki.stfu at gmail.com<mailto:ki.stfu at gmail.com>]
Sent: 03 December 2014 10:59
To: Abid, Hafiz
Cc: lldb-commits at cs.uiuc.edu<mailto:lldb-commits at cs.uiuc.edu>
Subject: Re: [Lldb-commits] [lldb] r223222 - Fix a hang on OSX while executing -exec-run.
Hello Abid,
Thanks for commit :)
But "ioctl" patch requires less changes than "select" version of this patch. Could you commit another patch to revert all unnecessary changes?
Thanks,
Ilia
On Wed, Dec 3, 2014 at 1:23 PM, Hafiz Abid Qadeer <hafiz_abid at mentor.com<mailto:hafiz_abid at mentor.com>> wrote:
Author: abidh
Date: Wed Dec 3 04:23:06 2014
New Revision: 223222
URL: http://llvm.org/viewvc/llvm-project?rev=223222&view=rev
Log:
Fix a hang on OSX while executing -exec-run.
Now we wait for input to become available before blocking in fgets.
More details on problem can be found in
http://lists.cs.uiuc.edu/pipermail/lldb-commits/Week-of-Mon-20141201/014290.html
Patch from dawn at burble.org<mailto:dawn at burble.org>.
Modified:
lldb/trunk/tools/lldb-mi/MICmnStreamStdin.cpp
lldb/trunk/tools/lldb-mi/MICmnStreamStdin.h
lldb/trunk/tools/lldb-mi/MICmnStreamStdinLinux.cpp
lldb/trunk/tools/lldb-mi/MICmnStreamStdinLinux.h
lldb/trunk/tools/lldb-mi/MIDriver.cpp
Modified: lldb/trunk/tools/lldb-mi/MICmnStreamStdin.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/tools/lldb-mi/MICmnStreamStdin.cpp?rev=223222&r1=223221&r2=223222&view=diff
==============================================================================
--- lldb/trunk/tools/lldb-mi/MICmnStreamStdin.cpp (original)
+++ lldb/trunk/tools/lldb-mi/MICmnStreamStdin.cpp Wed Dec 3 04:23:06 2014
@@ -434,3 +434,16 @@ CMICmnStreamStdin::SetOSStdinHandler(IOS
return MIstatus::success;
}
+
+//++ ------------------------------------------------------------------------------------
+// Details: Do some actions before exiting.
+// Type: Method.
+// Args: None.
+// Return: None.
+// Throws: None.
+//--
+void
+CMICmnStreamStdin::OnExitHandler(void)
+{
+ m_pStdinReadHandler->InterruptReadLine();
+}
Modified: lldb/trunk/tools/lldb-mi/MICmnStreamStdin.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/tools/lldb-mi/MICmnStreamStdin.h?rev=223222&r1=223221&r2=223222&view=diff
==============================================================================
--- lldb/trunk/tools/lldb-mi/MICmnStreamStdin.h (original)
+++ lldb/trunk/tools/lldb-mi/MICmnStreamStdin.h Wed Dec 3 04:23:06 2014
@@ -66,6 +66,7 @@ class CMICmnStreamStdin : public CMICmnB
public:
virtual bool InputAvailable(bool &vwbAvail) = 0;
virtual const MIchar *ReadLine(CMIUtilString &vwErrMsg) = 0;
+ virtual void InterruptReadLine(void){};
/* dtor */ virtual ~IOSStdinHandler(void){};
};
@@ -82,6 +83,7 @@ class CMICmnStreamStdin : public CMICmnB
void SetCtrlCHit(void);
bool SetVisitor(IStreamStdin &vrVisitor);
bool SetOSStdinHandler(IOSStdinHandler &vrHandler);
+ void OnExitHandler(void);
// Overridden:
public:
Modified: lldb/trunk/tools/lldb-mi/MICmnStreamStdinLinux.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/tools/lldb-mi/MICmnStreamStdinLinux.cpp?rev=223222&r1=223221&r2=223222&view=diff
==============================================================================
--- lldb/trunk/tools/lldb-mi/MICmnStreamStdinLinux.cpp (original)
+++ lldb/trunk/tools/lldb-mi/MICmnStreamStdinLinux.cpp Wed Dec 3 04:23:06 2014
@@ -22,7 +22,9 @@
// Third Party Headers:
#if !defined(_MSC_VER)
#include <sys/select.h>
+#include <unistd.h>
#include <termios.h>
+#include <sys/ioctl.h>
#endif // !defined( _MSC_VER )
#include <string.h> // For std::strerror()
@@ -153,30 +155,29 @@ CMICmnStreamStdinLinux::Shutdown(void)
bool
CMICmnStreamStdinLinux::InputAvailable(bool &vwbAvail)
{
- /* AD: Not used ATM but could come in handy just in case we need to do
- this, poll for input
-
- static const int STDIN = 0;
- static bool bInitialized = false;
-
- if( !bInitialized )
- {
- // Use termios to turn off line buffering
- ::termios term;
- ::tcgetattr( STDIN, &term );
- ::term.c_lflag &= ~ICANON;
- ::tcsetattr( STDIN, TCSANOW, &term );
- ::setbuf( stdin, NULL );
- bInitialized = true;
- }
-
- int nBytesWaiting;
- ::ioctl( STDIN, FIONREAD, &nBytesWaiting );
- vwbAvail = (nBytesWaiting > 0);
-
- return MIstatus::success;
- */
-
+#if !defined(_WIN32)
+ // The code below is needed on OSX where lldb-mi hangs when doing -exec-run.
+ // The hang seems to come from calling fgets and fileno from different thread.
+ // Although this problem was not observed on Linux.
+ // A solution based on 'select' was also proposed but it seems to slow things down
+ // a lot.
+ static bool bInitialized = false;
+
+ if (!bInitialized)
+ {
+ // Use termios to turn off line buffering
+ ::termios term;
+ ::tcgetattr(STDIN_FILENO, &term);
+ term.c_lflag &= ~ICANON;
+ ::tcsetattr(STDIN_FILENO, TCSANOW, &term);
+ ::setbuf(stdin, NULL);
+ bInitialized = true;
+ }
+
+ int nBytesWaiting;
+ ::ioctl(STDIN_FILENO, FIONREAD, &nBytesWaiting);
+ vwbAvail = (nBytesWaiting > 0);
+#endif
return MIstatus::success;
}
@@ -213,3 +214,16 @@ CMICmnStreamStdinLinux::ReadLine(CMIUtil
return pText;
}
+
+//++ ------------------------------------------------------------------------------------
+// Details: Interrupt current and prevent new ReadLine operations.
+// Type: Method.
+// Args: None.
+// Return: None.
+// Throws: None.
+//--
+void
+CMICmnStreamStdinLinux::InterruptReadLine(void)
+{
+ fclose(stdin);
+}
Modified: lldb/trunk/tools/lldb-mi/MICmnStreamStdinLinux.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/tools/lldb-mi/MICmnStreamStdinLinux.h?rev=223222&r1=223221&r2=223222&view=diff
==============================================================================
--- lldb/trunk/tools/lldb-mi/MICmnStreamStdinLinux.h (original)
+++ lldb/trunk/tools/lldb-mi/MICmnStreamStdinLinux.h Wed Dec 3 04:23:06 2014
@@ -51,6 +51,7 @@ class CMICmnStreamStdinLinux : public CM
// From CMICmnStreamStdin::IOSpecificReadStreamStdin
virtual bool InputAvailable(bool &vwbAvail);
virtual const MIchar *ReadLine(CMIUtilString &vwErrMsg);
+ virtual void InterruptReadLine(void);
// Methods:
private:
Modified: lldb/trunk/tools/lldb-mi/MIDriver.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/tools/lldb-mi/MIDriver.cpp?rev=223222&r1=223221&r2=223222&view=diff
==============================================================================
--- lldb/trunk/tools/lldb-mi/MIDriver.cpp (original)
+++ lldb/trunk/tools/lldb-mi/MIDriver.cpp Wed Dec 3 04:23:06 2014
@@ -1075,6 +1075,7 @@ CMIDriver::SetExitApplicationFlag(const
{
CMIUtilThreadLock lock(m_threadMutex);
m_bExitApp = true;
+ m_rStdin.OnExitHandler();
return;
}
@@ -1089,6 +1090,7 @@ CMIDriver::SetExitApplicationFlag(const
}
m_bExitApp = true;
+ m_rStdin.OnExitHandler();
}
//++ ------------------------------------------------------------------------------------
_______________________________________________
lldb-commits mailing list
lldb-commits at cs.uiuc.edu<mailto:lldb-commits at cs.uiuc.edu>
http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20141211/ea1cccfd/attachment.html>
More information about the lldb-commits
mailing list