Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

proto: optimize global (un)marshal lock using RWMutex #655

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

TennyZhuang
Copy link
Contributor

@TennyZhuang TennyZhuang commented Jan 6, 2020

This PR is also submitted to golang/protobuf#1004

This is a workaround to #656

Signed-off-by: TennyZhuang [email protected]

Thie PR Use RWMutex to optimize getMarshalInfo and getUnmarshalInfo, for these functions, only n (number of message type) will hit Write, and m(number of message) - n will hit Read, it's the best case to use RWMutex instead of Mutex.

This optimization introduce huge improvement in our scenario.

We have 1000 worker and 1 controller, and work and controller keep heartbeat with gRPC. They also exchange the job info with each other.

The message is like

message Job {
    uint64 job_id = 1;
    // Some other info
}

message Heartbeat {
    repeated Job jobs = 1;
}

About 10000 jobs in every Heartbeat, and the heartbeat QPS in controller is about 1000.

The controller handle the Heartbeat in about 10ms, and the network latency is about 10ms, but the client will use about 30s in maximum to finish a RPC call.

We use golang pprof block profile, and it seems that almost all block is caused by one global Mutex in protobuf package.

image

After optimization, in our use case, the rpc call from client will only use about 30ms, as our expected.

@TennyZhuang TennyZhuang changed the title proto: optimize global lock proto: optimize global (un)marshal lock using RWMutex Jan 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant