Skip to content
This repository has been archived by the owner on Aug 2, 2023. It is now read-only.

Delay between thread creation and thread created event is noticeably long #106

Closed
karthiknadig opened this issue Feb 23, 2018 · 7 comments
Closed

Comments

@karthiknadig
Copy link
Member

I added print('Created [%d]: %s' % (tid, pyd_tid)) to on_pydevd_thread_create to demonstrate this issue. Each of the worker threads here wait for 1 second before printing.

Worker 0
Worker2 0
done
Worker 1
Worker 4
Worker 2
Worker 3
Worker2 3
Worker2 4
Worker2 2
Worker2 1
Worker 0
done
Worker2 0
Worker2 2
Worker2 4
Worker2 3
Worker 3
Worker 1
Worker 2
Worker 4
Worker2 1
done
Worker 0
Worker2 0
Worker2 1
Worker2 2
Worker2 4
Worker 3
Worker 4
Worker2 3
Worker 2
Worker 1
done
Worker 0
Worker2 1
Worker2 0
Worker2 2
Worker2 4
Worker 3
Worker 4
Worker 2
Worker2 3
Worker 1
Created [1]: pid_14012_id_1394645891840
Created [2]: pid_14012_id_1394662689928
Created [3]: pid_14012_id_1394662689816
Created [4]: pid_14012_id_1394662690208
Created [5]: pid_14012_id_1394662690432
Created [6]: pid_14012_id_1394662690656
Created [7]: pid_14012_id_1394662731960
Created [8]: pid_14012_id_1394662732240
Created [9]: pid_14012_id_1394662732520
Created [10]: pid_14012_id_1394662732800
Created [11]: pid_14012_id_1394662733080
@int19h
Copy link
Contributor

int19h commented Feb 23, 2018

I wonder if what's happening here is that the worker threads are hammering CPU, and so the background thread that processes messages gets throttled down?

@int19h
Copy link
Contributor

int19h commented Feb 23, 2018

Although with a 1 sec delay this shouldn't be happening. But I'm generally curious whether the delay is in pydevd generating the message, or in our code receiving it.

@karthiknadig
Copy link
Member Author

This is the code that i was running to test:

import select
import threading
import sys
import time

def worker(i):
    while True:
        time.sleep(1)
        print('Worker {}'.format(i))

def worker2(i):
    while True:
        time.sleep(1)
        print('Worker2 {}'.format(i))

def main():
    workers = []
    for i in range(5):
        args=[i]
        t = threading.Thread(target=worker, args=args)
        workers.append(t)
        t.start()

    for i in range(5):
        args=[i]
        t = threading.Thread(target=worker2, args=args)
        workers.append(t)
        t.start()

    while True:
        time.sleep(1)
        print('done')

    input('Done')

if __name__ == '__main__':
    sys.exit(int(main() or 0))

@int19h int19h added the Bug label Feb 24, 2018
@huguesv huguesv added this to the Preview 2 milestone Mar 2, 2018
@DonJayamanne
Copy link
Contributor

@int19h @karthiknadig
Found the problem we're having with threads and the cause of the delays.

Problem:

  • PyDevd isn't reporting the threads as they are created because pydev doesn't support this today (I could be wrong)
  • Pydev seems to report thread only inside the message processing loop (it looks for new threads in that loop and reports them, hence the delay) (see [here https://github.com/Microsoft/ptvsd/blob/master/ptvsd/pydevd/pydevd.py#L512])

What about monkey patching, same as older version of PTVSD did?

  • Well, pydev does monkey patch the thread creation process
  • However we import threading early on and this breaks the code.
  • So we shouldn't be importing the threading module early, not until we monkey patch it

Fix

  • Even if we were to fix the monkey patching, I can't find code in pydev that reports the threads as they are created (need to spend some more time looking at the code).
  • Either way, I've made some changes to pydevd and it seems to be reporting threads as they get created.

So the real question is why isn't pydevd reporting threads as they are reported? Why isn't the monkey patching working

  • We've imported the threading namespace and monkey patching of the thread creation doesn't work in pydevd as a result of this.

  • Either way, we're going to have to make changes to ensure the monkey patching of the thread creation works correctly.

  • Once done, I think we can get live notification of thread creation.

Oh yes, finally we're going to have to have a very careful look at how we structure (have) the code.
We've imported a number of pydev modules in a number of places, and those in turn import the threading namespace.
I've had to modify __init__.py to ensure we patch the threads early on.

@DonJayamanne
Copy link
Contributor

DonJayamanne commented Apr 16, 2018

Unfortunately the threads reported contain the wrong names (Dummy-*), even though the ids are correct.
Guess the solution I had in plan will not work as the names aren't correct.

@DonJayamanne
Copy link
Contributor

DonJayamanne commented May 25, 2018

@karthiknadig
I'm no longer able to replicate the issue you have reported. Please could you test this out in VS.
FYI - I've tested this without the sleep as well.

I believe the recent changes made by @ericsnowcurrently may have resolved this. Previously we were importing the threading module before importing pydevd, which cause the monkey patching not to work, now we're importing pydevd first (in __init__.py), possible that has resolved it. (see my comments earlier)

@karthiknadig
Copy link
Member Author

Confirmed that this still exists, and the PR #430 fixes this.

karthiknadig pushed a commit that referenced this issue May 30, 2018
Fixes #106
* When a thread is created, notify about it as soon as possible.
* Properly mark internal threads as pydevd daemon threads.
Also fix tests to consider that the event related to thread
creation is given as the thread starts to run.
* Fix test flakiness and improve error message when messages are not equal.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

6 participants