这是indexloc提供的服务，不要输入任何密码
Skip to content
This repository was archived by the owner on Feb 16, 2023. It is now read-only.

Improve websocket performance #690

Merged
merged 7 commits into from
Aug 7, 2020
Merged

Improve websocket performance #690

merged 7 commits into from
Aug 7, 2020

Conversation

@IonesioJunior
Copy link
Member

@IonesioJunior IonesioJunior commented Aug 7, 2020

Description

Flask-Sockets uses gevent-websockets as a socket interface. This library has some performance issues. One simple way to improve its performance it's just overwriting the WebSocket mask_payload / unmask_payload methods by a new version using NumPy instead of standard python data-structures.

Checklist

@IonesioJunior IonesioJunior marked this pull request as ready for review August 7, 2020 14:22
@IonesioJunior IonesioJunior changed the title [WIP] Improve websocket performance Improve websocket performance Aug 7, 2020
@IonesioJunior IonesioJunior requested a review from LaRiffle August 7, 2020 14:22
@IonesioJunior IonesioJunior added Type: Improvement 📈 Performance improvement not introducing a new feature or requiring a major refactor Priority: 2 - High 😰 Should be fixed as quickly as possible, ideally within the current or following sprint Severity: 3 - Medium 😒 Does not cause a failure, impair usability, or interfere with the system labels Aug 7, 2020
LaRiffle
LaRiffle approved these changes Aug 7, 2020
View reviewed changes
Copy link
Contributor

@LaRiffle LaRiffle left a comment

Choose a reason for hiding this comment

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

I don't understand all the details but it looks good!
What's the current bandwidth you achieve with this?

@cereallarceny cereallarceny merged commit 695a041 into dev Aug 7, 2020
@cereallarceny cereallarceny deleted the improve_perform branch August 7, 2020 19:18
@IonesioJunior
Copy link
Member Author

IonesioJunior commented Aug 8, 2020

@LaRiffle Just giving a brief explanation.

Masking/Unmasking is a process used to guarantee some level of security during the transportãtion of the messages across proxies (as described in WebSocket RFC).

The problem: since the masking process needs to iterate over the message payload, the larger this message is, the longer it takes to process it.

The library we use (gevent-websocket) developed this process in order to use only native language structures, as we can see here.

Replacing these structures for NumPy structures should increase the performance.

Test code used:

import time
import torch as th
import syft as sy
from syft.grid.clients.data_centric_fl_client import DataCentricFLClient

hook = sy.TorchHook(th)

bob = DataCentricFLClient(hook, "ws://localhost:5000")

payload = 100
t = th.rand(1, int(payload * 10 ** 6 / 4))
timer = time.time()
t.send(bob)
delay = time.time - timer
print("bandwidth ", payload/delay)

Results:
I tested in a tiny Ubuntu Virtual Machine running in my notebook, so the bandwidths weren't that big. but the difference between them was significant.

Testing the nodes using this node version (Before):
before

Testing the nodes using this node version (After):
after

Conclusion: The improvement in my tests almost reached 25x. (which is pretty impressive but also a bit weird). So I'd like you to do your own tests just to check if it matches in your dev environment as well.

@nahuakang
Copy link
Member

nahuakang commented Aug 8, 2020

@IonesioJunior Noobie question here: I'm trying to run the test.py with a PyGrid Node manually running but I still get the following error. Any suggestion on how I might fix this and give it a try? 🙏

Traceback (most recent call last):
  File "test.py", line 7, in <module>
    bob = DataCentricFLClient(hook, "ws://locahost:5000")
  File "/home/nahua/projects/openmined/PySyft/syft/grid/clients/data_centric_fl_client.py", line 66, in __init__
    timeout,
  File "/home/nahua/projects/openmined/PySyft/syft/workers/websocket_client.py", line 66, in __init__
    self.connect()
  File "/home/nahua/projects/openmined/PySyft/syft/workers/websocket_client.py", line 78, in connect
    self.ws = websocket.create_connection(**args_)
  File "/home/nahua/anaconda3/envs/openmined/lib/python3.7/site-packages/websocket/_core.py", line 515, in create_connection
    websock.connect(url, **options)
  File "/home/nahua/anaconda3/envs/openmined/lib/python3.7/site-packages/websocket/_core.py", line 223, in connect
    options.pop('socket', None))
  File "/home/nahua/anaconda3/envs/openmined/lib/python3.7/site-packages/websocket/_http.py", line 114, in connect
    hostname, port, is_secure, proxy)
  File "/home/nahua/anaconda3/envs/openmined/lib/python3.7/site-packages/websocket/_http.py", line 158, in _get_addrinfo_list
    raise WebSocketAddressException(e)
websocket._exceptions.WebSocketAddressException: [Errno -2] Name or service not known

@LaRiffle
Copy link
Contributor

LaRiffle commented Aug 9, 2020

@IonesioJunior it's working like a charm, i have now a way faster execution <3

AmrMKayid added a commit that referenced this pull request Aug 14, 2020
@AmrMKayid
Merge branch 'dev' into abstract-core-classes
cd2551b 
* dev:
  [WIP] Update default ports (#691)
  Improve websocket performance (#690)
  Create Group model in Node DB (#688)
  Fix dataset search (#687)
  Add support for FSS online key generation for data centric workers (#686)
  Create User model in Network DB (#683)
  Create Role model in Node DB (#684)
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

2 more reviewers

@cereallarceny cereallarceny cereallarceny approved these changes

@LaRiffle LaRiffle LaRiffle approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

Priority: 2 - High 😰 Should be fixed as quickly as possible, ideally within the current or following sprint Severity: 3 - Medium 😒 Does not cause a failure, impair usability, or interfere with the system Type: Improvement 📈 Performance improvement not introducing a new feature or requiring a major refactor

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@IonesioJunior @nahuakang @LaRiffle @cereallarceny