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

Merge 'context' and 'client' #34

Merged
merged 2 commits into from
Nov 13, 2023
Merged

Conversation

lqxhub
Copy link
Collaborator

@lqxhub lqxhub commented Nov 12, 2023

把之前的ContextClient 这两个合并了,只保留了Client. 每次执行命令,通过一个client来传递 命令中的 上下文数据

把现阶段没有用到的一些功能先去掉(想的是先简化代码,后面根据需要再添加,不要一开始就是背负着历史包袱)

修改了Redis协议的解析和编码函数
简化数据处理流程,,现在去掉了C风格的处理命令的入口

这次改动有点大,步子迈的有点大,请帮忙多检查,多指正

像是接收到数据,解析Redis协议那部分, 我感觉改的还是不够好,但是感觉还是没有改到我理想的那样. 我原来想的是每个模块的功能划分好, 不要相互耦合, 现在 协议解析还是和 Client 有关联, 如果有好的实现方式,欢迎指教

ret_ = kNone;
}

inline std::string Message() const { return message_; };
Copy link
Collaborator

Choose a reason for hiding this comment

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

返回const &

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

好的. 我在写的时候,想的通过inline, 调用这个函数, 是不是和直接 .message_ 效果是一样的了, 所以就直接返回 string了

Copy link
Collaborator

Choose a reason for hiding this comment

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

好的. 我在写的时候,想的通过inline, 调用这个函数, 是不是和直接 .message_ 效果是一样的了, 所以就直接返回 string了

测试了一下目前这样写必定 copy,gcc version 8.4.1, O2优化。

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

好的. 我在写的时候,想的通过inline, 调用这个函数, 是不是和直接 .message_ 效果是一样的了, 所以就直接返回 string了

测试了一下目前这样写必定 copy,gcc version 8.4.1, O2优化。

多谢, 今晚回去改一下

@AlexStocks AlexStocks merged commit f5362b7 into OpenAtomFoundation:unstable Nov 13, 2023
1 check passed
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.

3 participants