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: add foreach for zero copy #66

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

Conversation

ccqy66
Copy link

@ccqy66 ccqy66 commented Nov 1, 2021

Reader 接口增加新api ForEachByte()
使用场景如下:

  • 等待特定字节到来。
  • 特定字节查询。

使用示例:

reader := connection.Reader()
if next := reader.ForEachByte(func(b byte) bool {
	return b == '\r'
}) ; next > 0{
	// do something
}

@Hchenn
Copy link
Contributor

Hchenn commented Nov 1, 2021

想先问下,这个 API 是为了解析 HTTP header “\r\n” 吗?

关于这个问题,我们有这样的想法:

view 1:

for each 效率太低,而 strings.Contains 等可以 SMID 加速,从这点看,重新实现 strings 下的方法并不划算。我们应该鼓励直接使用 strings API, 尽量不要重新实现。

按照这个逻辑,应该鼓励使用 Peek + strings API 来完成这类工作。而如果你看过 netpoll 代码的话,应该已经发现 Peek 可能因为跨节点,造成了 copy Peek。所以我们更倾向于解决 copy Peek, 而不是重新实现 strings API。

关于如何规避 copy Peek,我们也有一些草案,可以做到数据尽可能的做到在单节点 Peek

view 2:

Reader / Writer 接口天生有 rich 的倾向,但是不能放任 interface 变得越来越大。因此更想做的是把这些接口进行分类,定义为新的 rich interface, 比如 HTTPReader(仅举个例子)。。。

@Hchenn Hchenn added the doing label Nov 1, 2021
@ccqy66
Copy link
Author

ccqy66 commented Nov 1, 2021

1、 其实也是考虑到peek会有跨节点的问题。这样违背了zero copy的特性。这个放到下层去做可以避免copy的问题。
2、其实有些场景strings.contains也是实现不了的,就是得使用foreach的方式去做,比如协议定义了通过 '\r'做分隔,所以需要获取到第一个字符的位置(当然如果for each的效率低的话,可以考虑其他的算法)
3、view2 倒是一个理由,也是可以考虑哪些接口提供出去能够更好的简化上层的编码。

@Hchenn
Copy link
Contributor

Hchenn commented Nov 1, 2021

2、其实有些场景strings.contains也是实现不了的,就是得使用foreach的方式去做,比如协议定义了通过 '\r'做分隔,所以需要获取到第一个字符的位置(当然如果for each的效率低的话,可以考虑其他的算法)

strings 是一组 API 啊,你说的这个不就是 strings.Index

@ccqy66
Copy link
Author

ccqy66 commented Nov 1, 2021

嗯嗯,IndexByte也能实现,如果能解决跨节点的问题,这个目前来看是没有必要

@ccqy66
Copy link
Author

ccqy66 commented Nov 1, 2021

顺便问下,跨节点问题的草案是什么呢?

@Hchenn
Copy link
Contributor

Hchenn commented Nov 1, 2021

嗯嗯,IndexByte也能实现,如果能解决跨节点的问题,这个目前来看是没有必要

嗯,这个我先标记了。我们的方案已经在安排测试了,表现还可以,等 pr 了我 link 过来。

@Hchenn
Copy link
Contributor

Hchenn commented Nov 1, 2021

顺便问下,跨节点问题的草案是什么呢?

包大小预估:https://github.com/Hchenn/netpoll/pull/2/files

@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.


chenchen.ccqy66 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.

@joway
Copy link
Member

joway commented Dec 8, 2021

@Hchenn 如果用 Peek 来解析,只能用下面这种方法:

var skip int
for {
  buf := reader.Peek(reader.Len())
  n := bytes.IndexBytes(buf[skip:], '\n')
  if n < 0 { skip += len(buf[skip:]) }
}

一方面实现起来比较晦涩,另一方面由于真实的数据是一个二维数组,而如果等 peek return 后再去遍历一个一维切片,中间做了很多无用功。考虑到用特殊字符间隔的上层协议(http,mysql)其实挺多的,所以觉得还是有必要内置一个类似 bufio.ReadSilce 的接口:#91

@Hchenn Hchenn changed the title feat: add foreach for zero copy WIP: feat: add foreach for zero copy Apr 22, 2022
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.

None yet

4 participants