Enhancement: Improve GIL in ping.py Example

[acul71]

Description

The current implementation of the handle_ping function in ping.py is missing a break

Motivation

Performance analysis with py-spy indicates inefficiencies in the current implementation, specifically regarding GIL usage and active time.
Global Interpreter Lock (GIL) statistic provides insight into how much time Python's main thread spends holding the GIL during the execution of your program
Below is the py-spy output:

Collecting samples from 'python ping.py' (python v3.12.7)
Total Samples 8200
GIL: 99.00%, Active: 100.00%, Threads: 1

  %Own   %Total  OwnTime  TotalTime  Function (filename)                                                                                                      
 18.00%  21.00%   11.08s    18.50s   wrapper (trio/_core/_ki.py)
 17.00%  96.00%    8.75s    57.02s   unrolled_run (trio/_core/_run.py)
  4.00%   4.00%    5.74s     5.74s   get_events (trio/_core/_io_epoll.py)
  8.00%  49.00%    4.31s    33.15s   reset (libp2p/stream_muxer/mplex/mplex_stream.py)
 10.00%  20.00%    3.92s    13.77s   acquire (trio/_sync.py)
  9.00%  67.00%    3.47s    41.88s   handle_ping (chat/ping.py)
  6.00%   8.00%    3.24s     4.60s   read (libp2p/network/stream/net_stream.py)
  9.00%  10.00%    2.60s     4.91s   reschedule (trio/_core/_run.py)

The GIL percentage being at 99% and handle_ping consuming a significant portion of total active time highlights the need for optimization.

By adding a break, the new implementation minimizes unnecessary resource usage and enhances code robustness.

Current Implementation

async def handle_ping(stream: INetStream) -> None:
    while True:
        try:
            payload = await stream.read(PING_LENGTH)
            peer_id = stream.muxed_conn.peer_id
            if payload is not None:
                print(f"received ping from {peer_id}")

                await stream.write(payload)
                print(f"responded with pong to {peer_id}")

        except Exception:
            await stream.reset()

Are you planning to do it yourself in a pull request ?

Yes

[acul71]

@pacrob @dhuseby
I'm adding a break like this:

async def handle_ping(stream: INetStream) -> None:
    while True:
        try:
            with trio.fail_after(RESP_TIMEOUT):
                payload = await stream.read(PING_LENGTH)

            if payload:
                peer_id = stream.muxed_conn.peer_id
                print(f"received ping from {peer_id}")
                await stream.write(payload)
                print(f"responded with pong to {peer_id}")

        except Exception as e:
            # Add debugging information for diagnosis
            print(f"Error in handle_ping: {e.__class__.__name__}: {e}")
            await stream.reset()
            break

I'm still studying trio lib and py-libp2p lib: Is it normal to have a Exception when a stream is closed?

PR coming soon!

python ping.py
Run this from the same folder in another console:

python ping.py -p 8001 -d /ip4/127.0.0.1/tcp/8000/p2p/QmcbjN8opPEJe55T2Pe3nJSoJjbNU7GVWhU1oCF7NW7GMa

Waiting for incoming connection...
received ping from QmUUrkzunoJfkZQdN3Fjq1dXzJknPhqK5hZmEhjjasyGTn
responded with pong to QmUUrkzunoJfkZQdN3Fjq1dXzJknPhqK5hZmEhjjasyGTn
Error in handle_ping: StreamReset: 

PS: GIL is now normal

Collecting samples from 'python ping.py' (python v3.12.7)
Total Samples 45400
GIL: 0.00%, Active: 5.00%, Threads: 1

  %Own   %Total  OwnTime  TotalTime  Function (filename)                                                                                                      
  5.00%   5.00%   14.42s    14.42s   get_events (trio/_core/_io_epoll.py)
  0.00%   0.00%   0.010s    0.010s   _convertTag (Crypto/Util/asn1.py)
  0.00%   0.00%   0.010s    0.040s   unrolled_run (trio/_core/_run.py)
  0.00%   0.00%   0.010s    0.010s   _close (trio/_core/_run.py)
  0.00%   0.00%   0.010s    0.010s   reschedule (trio/_core/_run.py)
  0.00%   0.00%   0.000s    0.010s   upgrade_security (libp2p/transport/upgrader.py)
  0.00%   0.00%   0.000s    0.010s   _create_subject_public_key_info (Crypto/PublicKey/__init__.py)
  0.00%   0.00%   0.000s    0.010s   __init__ (Crypto/Util/asn1.py)
  0.00%   0.00%   0.000s    0.010s   from_pubkey (libp2p/peer/id.py)
  0.00%   5.00%   0.000s    14.46s   main (chat/ping.py)
  0.00%   0.00%   0.000s    0.010s   run_handshake (libp2p/security/insecure/transport.py)
  0.00%   0.00%   0.000s    0.010s   secure_inbound (libp2p/security/insecure/transport.py)
  0.00%   0.00%   0.000s    0.010s   serialize (libp2p/crypto/keys.py)
  0.00%   0.00%   0.000s    0.010s   run (libp2p/tools/async_service/trio_service.py)
  0.00%   0.00%   0.000s    0.010s   _run_and_manage_task (libp2p/tools/async_service/base.py)
  0.00%   0.00%   0.000s    0.010s   make_exchange_message (libp2p/security/insecure/transport.py)
  0.00%   0.00%   0.000s    0.010s   __exit__ (trio/_core/_run.py)
  0.00%   5.00%   0.000s    14.46s   <module> (chat/ping.py)
  0.00%   0.00%   0.000s    0.010s   secure_inbound (libp2p/security/security_multistream.py)
  0.00%   0.00%   0.000s    0.010s   to_bytes (libp2p/crypto/rsa.py)
  0.00%   0.00%   0.000s    0.010s   wrapper (trio/_core/_ki.py)
  0.00%   0.00%   0.000s    0.010s   export_key (Crypto/PublicKey/RSA.py)
  0.00%   0.00%   0.000s    0.010s   conn_handler (libp2p/network/swarm.py)
  0.00%   0.00%   0.000s    0.010s   handler (libp2p/transport/tcp/tcp.py)
  0.00%   0.00%   0.000s    0.010s   _run_handler (trio/_highlevel_serve_listeners.py)
  0.00%   5.00%   0.000s    14.46s   run (trio/_core/_run.py)
  0.00%   0.00%   0.000s    0.010s   _serialize_to_protobuf (libp2p/crypto/keys.py)

[acul71]

The ping demo is in reality the echo demo
https://py-libp2p.readthedocs.io/en/stable/examples.ping.html
I remember we talked about this in last py-libp2p meeting, but now I don't remember exactly was something to do about current and stable versions....
Not sure how this documentation is (auto?) generated, will take a look in the next days.
If you feel like give some hint how to work to "fix" that.

[pacrob]

@acul71 I've pushed a release, so the issue discussed of the ping demo not being available should be moot.

I believe that the ping demo structure was copied from the existing echo demo in #477

As for how to fix, not sure 😅 But looking at it! I'm new to trio as well, so don't know if it's normal to have an Exception when the stream is closed.

[endomorphosis]

https://peps.python.org/pep-0703/

I looked into this yesterday, I will keep you all updated if i find anything noteworthy out.