|
|
# 代码审查与优化建议
|
|
|
|
|
|
## 1. 与互联网最佳实践的差别
|
|
|
|
|
|
### 1.1 gnet框架最佳实践
|
|
|
|
|
|
#### ❌ 问题1:未充分利用 `Conn.Context()` 存储连接数据
|
|
|
|
|
|
**当前实现**:
|
|
|
```go
|
|
|
// internal/server/server.go:477
|
|
|
func (h *eventHandler) OnTraffic(c gnet.Conn) (action gnet.Action) {
|
|
|
connID := c.RemoteAddr().String()
|
|
|
connInterface, err := h.connManager.Get(connID) // 每次都从管理器获取
|
|
|
// ...
|
|
|
}
|
|
|
```
|
|
|
|
|
|
**最佳实践**:gnet推荐使用 `Conn.Context()` 为每个连接存储必要的资源,避免跨连接共享资源导致的竞争。
|
|
|
|
|
|
**建议改进**:
|
|
|
```go
|
|
|
// 在OnOpen时设置Context
|
|
|
func (h *eventHandler) OnOpen(c gnet.Conn) ([]byte, gnet.Action) {
|
|
|
connID := c.RemoteAddr().String()
|
|
|
conn := connection.NewConnection(connID, c)
|
|
|
|
|
|
// 将连接数据存储到gnet的Context中
|
|
|
connData := &connectionData{
|
|
|
conn: conn,
|
|
|
unpacker: nil, // 延迟创建
|
|
|
}
|
|
|
c.SetContext(connData)
|
|
|
|
|
|
// ...
|
|
|
}
|
|
|
|
|
|
// 在OnTraffic中直接使用
|
|
|
func (h *eventHandler) OnTraffic(c gnet.Conn) (action gnet.Action) {
|
|
|
connData := c.Context().(*connectionData) // 直接获取,无需查找
|
|
|
// ...
|
|
|
}
|
|
|
```
|
|
|
|
|
|
#### ❌ 问题2:未使用 `AsyncWrite` 进行异步写入
|
|
|
|
|
|
**当前实现**:
|
|
|
```go
|
|
|
// internal/connection/connection.go:52
|
|
|
func (c *Connection) Write(data []byte) error {
|
|
|
_, err := c.conn.Write(data) // 同步写入
|
|
|
return err
|
|
|
}
|
|
|
```
|
|
|
|
|
|
**最佳实践**:gnet推荐使用 `AsyncWrite` 进行异步写入,避免阻塞事件循环。
|
|
|
|
|
|
**建议改进**:
|
|
|
```go
|
|
|
func (c *Connection) Write(data []byte) error {
|
|
|
// 复制数据,因为AsyncWrite是异步的
|
|
|
buf := make([]byte, len(data))
|
|
|
copy(buf, data)
|
|
|
|
|
|
_, err := c.conn.AsyncWrite(buf, func(c gnet.Conn, err error) error {
|
|
|
if err != nil {
|
|
|
// 处理写入错误
|
|
|
}
|
|
|
return nil
|
|
|
})
|
|
|
return err
|
|
|
}
|
|
|
```
|
|
|
|
|
|
#### ❌ 问题3:未启用 gnet 的优化标签
|
|
|
|
|
|
**当前实现**:未在构建时启用 `poll_opt` 和 `gc_opt` 标签。
|
|
|
|
|
|
**建议改进**:
|
|
|
- 在构建时添加 `-tags=poll_opt` 启用优化的轮询器
|
|
|
- 在构建时添加 `-tags=gc_opt` 启用优化的连接存储
|
|
|
|
|
|
#### ⚠️ 问题4:数据损坏风险
|
|
|
|
|
|
**当前实现**:
|
|
|
```go
|
|
|
// internal/server/server.go:496
|
|
|
data, _ := c.Peek(-1) // 获取数据
|
|
|
// ... 处理数据
|
|
|
c.Discard(totalProcessed) // 丢弃数据
|
|
|
```
|
|
|
|
|
|
**风险**:如果处理过程中数据被修改,可能导致数据损坏。
|
|
|
|
|
|
**建议**:确保 `Peek` 返回的数据在使用前不被修改,或在 `Discard` 后不再使用。
|
|
|
|
|
|
### 1.2 网络库通用最佳实践
|
|
|
|
|
|
#### ❌ 问题5:缺少连接限流和背压机制
|
|
|
|
|
|
**当前实现**:没有对连接数或消息处理速度进行限流。
|
|
|
|
|
|
**建议**:
|
|
|
- 实现连接数限流
|
|
|
- 实现消息处理速率限流
|
|
|
- 实现背压机制(当处理速度跟不上接收速度时)
|
|
|
|
|
|
#### ❌ 问题6:缺少优雅关闭机制
|
|
|
|
|
|
**当前实现**:`Stop()` 方法可能立即关闭所有连接。
|
|
|
|
|
|
**建议**:
|
|
|
- 实现优雅关闭:等待正在处理的请求完成
|
|
|
- 停止接受新连接
|
|
|
- 设置关闭超时
|
|
|
|
|
|
## 2. 代码设计和架构设计的不足
|
|
|
|
|
|
### 2.1 Unpacker Buffer 无限增长问题
|
|
|
|
|
|
**严重问题**:
|
|
|
```go
|
|
|
// internal/unpacker/length_field.go:36
|
|
|
func (u *lengthFieldUnpacker) Unpack(data []byte) ([][]byte, []byte, error) {
|
|
|
u.buffer = append(u.buffer, data...) // 可能无限增长
|
|
|
// ...
|
|
|
}
|
|
|
```
|
|
|
|
|
|
**问题**:
|
|
|
- 如果客户端发送恶意数据(如非常大的长度字段),buffer可能无限增长
|
|
|
- 没有最大buffer大小限制
|
|
|
- 可能导致内存耗尽
|
|
|
|
|
|
**建议改进**:
|
|
|
```go
|
|
|
const (
|
|
|
MaxUnpackerBufferSize = 10 * 1024 * 1024 // 10MB
|
|
|
)
|
|
|
|
|
|
func (u *lengthFieldUnpacker) Unpack(data []byte) ([][]byte, []byte, error) {
|
|
|
// 检查buffer大小
|
|
|
if len(u.buffer)+len(data) > MaxUnpackerBufferSize {
|
|
|
return nil, nil, errors.New("unpacker buffer size exceeded")
|
|
|
}
|
|
|
|
|
|
u.buffer = append(u.buffer, data...)
|
|
|
// ...
|
|
|
}
|
|
|
```
|
|
|
|
|
|
### 2.2 连接管理器的锁粒度问题
|
|
|
|
|
|
**当前实现**:
|
|
|
```go
|
|
|
// internal/connection/manager.go:28
|
|
|
func (m *Manager) Add(conn ConnectionInterface) error {
|
|
|
m.mu.Lock() // 全局锁
|
|
|
defer m.mu.Unlock()
|
|
|
// ...
|
|
|
}
|
|
|
```
|
|
|
|
|
|
**问题**:所有连接操作都使用同一个锁,可能导致锁竞争。
|
|
|
|
|
|
**建议**:使用分段锁(sharded locks)或读写锁优化。
|
|
|
|
|
|
### 2.3 缺少对象池
|
|
|
|
|
|
**当前实现**:频繁创建和销毁对象,增加GC压力。
|
|
|
|
|
|
**建议**:
|
|
|
```go
|
|
|
var (
|
|
|
messagePool = sync.Pool{
|
|
|
New: func() interface{} {
|
|
|
return make([]byte, 0, 4096)
|
|
|
},
|
|
|
}
|
|
|
|
|
|
contextPool = sync.Pool{
|
|
|
New: func() interface{} {
|
|
|
return &Context{}
|
|
|
},
|
|
|
}
|
|
|
)
|
|
|
```
|
|
|
|
|
|
### 2.4 错误处理不够完善
|
|
|
|
|
|
**当前实现**:
|
|
|
```go
|
|
|
// internal/server/server.go:533
|
|
|
messages, remaining, err := unpacker.Unpack(data)
|
|
|
if err != nil {
|
|
|
h.logger.Error("Unpack error: %v", err)
|
|
|
c.Discard(inBuffer) // 直接丢弃所有数据
|
|
|
return gnet.None
|
|
|
}
|
|
|
```
|
|
|
|
|
|
**问题**:
|
|
|
- 错误处理过于简单
|
|
|
- 没有区分不同类型的错误(可恢复 vs 不可恢复)
|
|
|
- 没有错误重试机制
|
|
|
|
|
|
**建议**:实现更细粒度的错误处理和恢复机制。
|
|
|
|
|
|
### 2.5 缺少超时控制
|
|
|
|
|
|
**当前实现**:没有对消息处理设置超时。
|
|
|
|
|
|
**建议**:
|
|
|
- 为每个消息处理设置超时
|
|
|
- 使用 `context.WithTimeout` 控制处理时间
|
|
|
- 超时后自动取消处理
|
|
|
|
|
|
## 3. 内存优化
|
|
|
|
|
|
### 3.1 频繁的 []byte 分配
|
|
|
|
|
|
**问题**:
|
|
|
```go
|
|
|
// internal/server/server.go:564
|
|
|
for _, message := range messages {
|
|
|
ctx := createContext(...) // 每次创建新Context
|
|
|
// ...
|
|
|
}
|
|
|
```
|
|
|
|
|
|
**优化建议**:
|
|
|
1. 使用 `sync.Pool` 复用对象
|
|
|
2. 使用对象池管理 Context、Request、Response
|
|
|
3. 减少不必要的内存拷贝
|
|
|
|
|
|
### 3.2 Unpacker Buffer 优化
|
|
|
|
|
|
**问题**:每次 `append` 可能导致底层数组重新分配。
|
|
|
|
|
|
**优化建议**:
|
|
|
```go
|
|
|
type lengthFieldUnpacker struct {
|
|
|
buffer []byte
|
|
|
bufferCap int // 记录容量
|
|
|
}
|
|
|
|
|
|
func (u *lengthFieldUnpacker) Unpack(data []byte) ([][]byte, []byte, error) {
|
|
|
// 预分配足够的容量
|
|
|
if cap(u.buffer) < len(u.buffer)+len(data) {
|
|
|
newCap := (len(u.buffer) + len(data)) * 2
|
|
|
newBuffer := make([]byte, len(u.buffer), newCap)
|
|
|
copy(newBuffer, u.buffer)
|
|
|
u.buffer = newBuffer
|
|
|
}
|
|
|
|
|
|
u.buffer = append(u.buffer, data...)
|
|
|
// ...
|
|
|
}
|
|
|
```
|
|
|
|
|
|
### 3.3 消息处理中的多次拷贝
|
|
|
|
|
|
**问题**:
|
|
|
```go
|
|
|
// internal/server/server.go:496
|
|
|
data, _ := c.Peek(-1) // 第一次:从gnet buffer获取
|
|
|
// ...
|
|
|
message := make([]byte, end-start) // 第二次:从unpacker buffer拷贝
|
|
|
copy(message, u.buffer[start:end])
|
|
|
```
|
|
|
|
|
|
**优化建议**:
|
|
|
- 尽量减少不必要的拷贝
|
|
|
- 使用零拷贝技术(如果可能)
|
|
|
- 复用缓冲区
|
|
|
|
|
|
### 3.4 连接属性的内存占用
|
|
|
|
|
|
**问题**:
|
|
|
```go
|
|
|
// internal/connection/connection.go:21
|
|
|
attributes map[string]interface{} // 可能存储大量数据
|
|
|
```
|
|
|
|
|
|
**优化建议**:
|
|
|
- 限制attributes的大小
|
|
|
- 使用更紧凑的数据结构
|
|
|
- 定期清理不使用的attributes
|
|
|
|
|
|
## 4. gnet功能使用情况
|
|
|
|
|
|
### 4.1 未使用的功能
|
|
|
|
|
|
#### ❌ Conn.Context() 未使用
|
|
|
- **当前**:使用独立的连接管理器
|
|
|
- **建议**:使用 `Conn.Context()` 存储连接数据
|
|
|
|
|
|
#### ❌ AsyncWrite 未使用
|
|
|
- **当前**:使用同步 `Write`
|
|
|
- **建议**:使用 `AsyncWrite` 进行异步写入
|
|
|
|
|
|
#### ❌ 优化标签未启用
|
|
|
- **当前**:未启用 `poll_opt` 和 `gc_opt`
|
|
|
- **建议**:在构建时启用这些标签
|
|
|
|
|
|
#### ❌ 未使用 gnet 的缓冲区管理
|
|
|
- **当前**:自己管理缓冲区
|
|
|
- **建议**:充分利用 gnet 的缓冲区管理功能
|
|
|
|
|
|
### 4.2 可以改进的地方
|
|
|
|
|
|
1. **使用 gnet 的连接池**:如果gnet提供连接池功能,应该使用
|
|
|
2. **利用 gnet 的统计信息**:使用gnet提供的连接统计信息
|
|
|
3. **使用 gnet 的插件系统**:如果gnet有插件系统,应该利用
|
|
|
|
|
|
## 5. 第三方使用便利性
|
|
|
|
|
|
### 5.1 API设计问题
|
|
|
|
|
|
#### ⚠️ 问题1:API不够直观
|
|
|
|
|
|
**当前**:
|
|
|
```go
|
|
|
server, err := nnet.NewServer(cfg)
|
|
|
server.Router().GET("/path", handler)
|
|
|
```
|
|
|
|
|
|
**建议**:提供更简洁的API
|
|
|
```go
|
|
|
server := nnet.NewServer(cfg)
|
|
|
server.GET("/path", handler) // 直接调用,无需获取Router
|
|
|
```
|
|
|
|
|
|
#### ⚠️ 问题2:错误信息不够友好
|
|
|
|
|
|
**当前**:错误信息可能不够详细,不利于调试。
|
|
|
|
|
|
**建议**:
|
|
|
- 提供详细的错误信息
|
|
|
- 包含上下文信息(如连接ID、请求数据等)
|
|
|
- 提供错误码和错误分类
|
|
|
|
|
|
#### ⚠️ 问题3:配置过于复杂
|
|
|
|
|
|
**当前**:配置项较多,可能让用户困惑。
|
|
|
|
|
|
**建议**:
|
|
|
- 提供合理的默认值
|
|
|
- 提供配置验证和提示
|
|
|
- 提供配置示例
|
|
|
|
|
|
### 5.2 文档和示例
|
|
|
|
|
|
#### ❌ 缺少完整的文档
|
|
|
- 缺少API文档
|
|
|
- 缺少使用指南
|
|
|
- 缺少最佳实践文档
|
|
|
|
|
|
#### ❌ 缺少示例代码
|
|
|
- 缺少基本使用示例
|
|
|
- 缺少高级功能示例
|
|
|
- 缺少常见场景示例
|
|
|
|
|
|
### 5.3 可扩展性
|
|
|
|
|
|
#### ⚠️ 问题:扩展点不够清晰
|
|
|
|
|
|
**建议**:
|
|
|
- 明确标识可扩展点
|
|
|
- 提供扩展接口文档
|
|
|
- 提供扩展示例
|
|
|
|
|
|
## 6. 优先级改进建议
|
|
|
|
|
|
### 高优先级(必须修复)
|
|
|
|
|
|
1. **Unpacker Buffer 大小限制**:防止内存耗尽
|
|
|
2. **使用 Conn.Context()**:提高性能和正确性
|
|
|
3. **使用 AsyncWrite**:避免阻塞事件循环
|
|
|
4. **错误处理改进**:提高系统稳定性
|
|
|
|
|
|
### 中优先级(建议修复)
|
|
|
|
|
|
1. **对象池**:减少GC压力
|
|
|
2. **连接管理器优化**:减少锁竞争
|
|
|
3. **超时控制**:防止资源泄漏
|
|
|
4. **优雅关闭**:提高可用性
|
|
|
|
|
|
### 低优先级(可选优化)
|
|
|
|
|
|
1. **API简化**:提高易用性
|
|
|
2. **文档完善**:提高可维护性
|
|
|
3. **示例代码**:提高学习曲线
|
|
|
4. **性能优化**:进一步提升性能
|
|
|
|
|
|
## 7. 具体改进代码示例
|
|
|
|
|
|
### 7.1 使用 Conn.Context()
|
|
|
|
|
|
```go
|
|
|
type connectionData struct {
|
|
|
conn *connection.Connection
|
|
|
unpacker unpackerpkg.Unpacker
|
|
|
protocol protocolpkg.Protocol
|
|
|
}
|
|
|
|
|
|
func (h *eventHandler) OnOpen(c gnet.Conn) ([]byte, gnet.Action) {
|
|
|
connID := c.RemoteAddr().String()
|
|
|
conn := connection.NewConnection(connID, c)
|
|
|
|
|
|
connData := &connectionData{
|
|
|
conn: conn,
|
|
|
}
|
|
|
c.SetContext(connData)
|
|
|
|
|
|
// ...
|
|
|
}
|
|
|
|
|
|
func (h *eventHandler) OnTraffic(c gnet.Conn) (action gnet.Action) {
|
|
|
connData := c.Context().(*connectionData)
|
|
|
// 直接使用connData,无需查找
|
|
|
// ...
|
|
|
}
|
|
|
```
|
|
|
|
|
|
### 7.2 添加Buffer大小限制
|
|
|
|
|
|
```go
|
|
|
type lengthFieldUnpacker struct {
|
|
|
// ...
|
|
|
maxBufferSize int
|
|
|
}
|
|
|
|
|
|
func NewLengthFieldUnpacker(config unpackerpkg.LengthFieldUnpacker) unpackerpkg.Unpacker {
|
|
|
return &lengthFieldUnpacker{
|
|
|
// ...
|
|
|
maxBufferSize: 10 * 1024 * 1024, // 10MB
|
|
|
}
|
|
|
}
|
|
|
|
|
|
func (u *lengthFieldUnpacker) Unpack(data []byte) ([][]byte, []byte, error) {
|
|
|
if len(u.buffer)+len(data) > u.maxBufferSize {
|
|
|
return nil, nil, errors.New("unpacker buffer size exceeded")
|
|
|
}
|
|
|
// ...
|
|
|
}
|
|
|
```
|
|
|
|
|
|
### 7.3 使用对象池
|
|
|
|
|
|
```go
|
|
|
var (
|
|
|
contextPool = sync.Pool{
|
|
|
New: func() interface{} {
|
|
|
return &Context{}
|
|
|
},
|
|
|
}
|
|
|
|
|
|
messagePool = sync.Pool{
|
|
|
New: func() interface{} {
|
|
|
return make([]byte, 0, 4096)
|
|
|
},
|
|
|
}
|
|
|
)
|
|
|
|
|
|
func getContext() *Context {
|
|
|
return contextPool.Get().(*Context)
|
|
|
}
|
|
|
|
|
|
func putContext(ctx *Context) {
|
|
|
ctx.Reset() // 重置状态
|
|
|
contextPool.Put(ctx)
|
|
|
}
|
|
|
```
|
|
|
|
|
|
## 8. 总结
|
|
|
|
|
|
当前实现已经具备了基本功能,但在以下方面还有改进空间:
|
|
|
|
|
|
1. **性能优化**:充分利用gnet的特性,减少内存分配
|
|
|
2. **稳定性**:添加错误处理、超时控制、资源限制
|
|
|
3. **易用性**:简化API,完善文档和示例
|
|
|
4. **可维护性**:改进代码结构,提高可读性
|
|
|
|
|
|
建议按照优先级逐步改进,先解决高优先级问题,再优化中低优先级问题。
|
|
|
|