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

WIP: feat: io_uring for netpoll I/O poller #197

Open
wants to merge 67 commits into
base: develop
Choose a base branch
from

Conversation

Jacob953
Copy link

@Jacob953 Jacob953 commented Aug 25, 2022

Follows #151, tracing at #194. Hold, please ;)

@Hchenn Hchenn added the feature label Aug 25, 2022

// If the ring is initialized with IORING_SETUP_CQE32, then this field
// contains 16-bytes of padding, doubling the size of the CQE.
BigCQE []uint64
Copy link
Contributor

Choose a reason for hiding this comment

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

16bytes 应该是 [2]uint64 吧,这样定义怎么初始化呢

Len uint32 // buffer size or number of iovecs
OpcodeFlags uint32
UserData uint64 // data to be passed back at completion time

Copy link
Contributor

Choose a reason for hiding this comment

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

以下先 pad [3]uint64 吧,这里 union 先不区分

Off uint64 // offset into file
Addr uint64 // pointer to buffer or iovecs
Len uint32 // buffer size or number of iovecs
OpcodeFlags uint32
Copy link
Contributor

Choose a reason for hiding this comment

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

this is UnionFlags

// sysMmap is used to configure the URingSQE and URingCQE,
// it should only be called after the sysSetUp function has completed successfully.
func (u *URing) sysMmap(p *ringParams) (err error) {
size := unsafe.Sizeof(URingCQE{})
Copy link
Contributor

Choose a reason for hiding this comment

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

sizeof 定义为 global var/const,不用每次都算

}

// TODO: syscall.MAP_POPULATE unsupport for macox
data, err := syscall.Mmap(u.fd, 0, int(u.sqRing.ringSize), syscall.PROT_READ|syscall.PROT_WRITE, syscall.MAP_SHARED)
Copy link
Contributor

Choose a reason for hiding this comment

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

syscall.Mmap/Unmap 封装一下,以后可以修改

Copy link
Contributor

Choose a reason for hiding this comment

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

uring 不是 only linux 吗,为什么考虑 macox ? 这里没有 syscall.MAP_POPULATE 不行的

"syscall"
"unsafe"
)

Copy link
Contributor

Choose a reason for hiding this comment

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

public 这个文件的所有方法,sdk 应该支持直接 syscall

uring/syscall.go Outdated
}

//go:linkname sockaddr syscall.Sockaddr.sockaddr
func sockaddr(addr syscall.Sockaddr) (unsafe.Pointer, uint32, error)
Copy link
Contributor

Choose a reason for hiding this comment

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

没看到使用地方,先从这里移出吧,不是一个分类

uring/uring.go Outdated
}

// IOURing create new io_uring instance with Setup Options
func IOURing(entries uint32, ops ...setupOp) (r *URing, err error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

入口文件,请把用户能调的放在最前面,私有内容放后面,或者放其他文件里

uring/uring.go Outdated
}

// Init system call numbers
const (
Copy link
Contributor

Choose a reason for hiding this comment

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

这些 const 扔到 syscall.go 里

}

// GetEventsArg implements URing
func GetEventsArg(sigMask uintptr, sigMaskSz uint32, ts uintptr) *getEventsArg {
Copy link
Contributor

Choose a reason for hiding this comment

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

global func 别混在这里,如果是 private, 小写放最后;public 就放其他文件归类

@@ -25,7 +25,10 @@ import (
"unsafe"
)

func openPoll() Poll {
Copy link
Contributor

Choose a reason for hiding this comment

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

poll_xxx 这些先删掉吧,这里需要开发注册机制,不通过 type 区分

poll_io_uring.go Outdated

package netpoll

import uring "github.com/cloudwego/netpoll/io_uring"
Copy link
Contributor

Choose a reason for hiding this comment

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

路径错误

poll_io_uring.go Outdated
@@ -0,0 +1,45 @@
// Copyright 2021 CloudWeGo Authors
Copy link
Contributor

Choose a reason for hiding this comment

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

Copyright 2022 CloudWeGo Authors

@Jacob953
Copy link
Author

Run go test -bench=".*" -benchtime=1000000x at uring/benchmark/ .

goos: linux
goarch: arm64
pkg: github.com/cloudwego/netpoll/uring/benchmark
BenchmarkEpoll-4   	 1000000	      1232 ns/op
BenchmarkUring-4   	 1000000	       457.4 ns/op
PASS

@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.


Jacob953 seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging this pull request may close these issues.

4 participants