AlexStocks commented on code in PR #3305: URL: https://github.com/apache/dubbo-go/pull/3305#discussion_r3288452614
########## cluster/router/chain/cache.go: ########## @@ -0,0 +1,92 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package chain + +import ( + "sync" +) + +import ( + "dubbo.apache.org/dubbo-go/v3/cluster/router" + "dubbo.apache.org/dubbo-go/v3/protocol/base" +) + +// routerCache implements router.Cache by storing pre-computed AddrPool +// keyed by Poolable.Name(). It is rebuilt in its entirety on each +// SetInvokers call so that bitmap indices stay aligned with the current +// invoker snapshot. +type routerCache struct { + mu sync.RWMutex + invokers []base.Invoker + pools map[string]*poolEntry +} + +type poolEntry struct { + pool router.AddrPool +} + +func newRouterCache() *routerCache { + return &routerCache{ + pools: make(map[string]*poolEntry), + } +} + +func (c *routerCache) GetInvokers() []base.Invoker { + c.mu.RLock() + defer c.mu.RUnlock() + ret := make([]base.Invoker, len(c.invokers)) + copy(ret, c.invokers) + return ret +} + +func (c *routerCache) FindAddrPool(p router.Poolable) (router.AddrPool, []base.Invoker) { + c.mu.RLock() + defer c.mu.RUnlock() + entry, ok := c.pools[p.Name()] + if !ok { + return nil, nil + } + invokers := make([]base.Invoker, len(c.invokers)) + copy(invokers, c.invokers) + return entry.pool, invokers +} + +// FindAddrMeta is a no-op, reserved for future use. +func (c *routerCache) FindAddrMeta(p router.Poolable) router.AddrMetadata { + return nil +} + +// rebuild iterates all Poolable routers whose ShouldPool returns true, +// calls Pool on each with current invokers, and atomically swaps in the +// new pools under write lock. +func (c *routerCache) rebuild(invokers []base.Invoker, routers []router.PriorityRouter) { + newPools := make(map[string]*poolEntry, len(routers)) + for _, r := range routers { + p, ok := r.(router.Poolable) + if !ok || !p.ShouldPool() { + continue + } Review Comment: [P0] `Pool()` 返回 `(AddrPool, AddrMetadata)`，但第二个返回值被 `pool, _ := p.Pool(invokers)` 静默丢弃。如果未来有路由器使用 AddrMetadata 传递关键索引元数据，cache 层将完全丢失该信息，且无任何注释说明为何忽略。 建议：存储第二个返回值到 poolEntry 中，添加 `meta router.AddrMetadata` 字段；如当前确无用途，至少添加 `// TODO: store AddrMetadata when needed` 注释。 ########## cluster/router/tag/router.go: ########## @@ -52,6 +57,16 @@ func (p *PriorityRouter) Route(invokers []base.Invoker, url *common.URL, invocat logger.Warnf("[tag router] invokers from previous router is empty") Review Comment: [P0] TagRouter 并发读写 p.cache 无同步保护。p.cache 在 SetCache() 中写入（持有 RouterChain.mutex 写锁），在 Route() 中读取（可能仅持读锁）。p.cache 是接口类型指针赋值，Go 内存模型不保证指针赋值的原子性，且 `p.cache != nil` 检查与后续 `p.cache.FindAddrPool()` 之间存在 TOCTOU 竞态。 建议：将 p.cache 改为 `atomic.Value` 或使用 `sync.RWMutex` 保护，确保 SetCache 和 Route 中的读取同步。 ########## cluster/router/tag/router.go: ########## @@ -157,3 +172,269 @@ func parseRoute(routeContent string) (*global.RouterConfig, error) { } return routerConfig, nil } + +func (p *PriorityRouter) Name() string { + return "tag" +} + +func (p *PriorityRouter) ShouldPool() bool { + return true +} + +func (p *PriorityRouter) Pool(invokers []base.Invoker) (router.AddrPool, router.AddrMetadata) { + pool := make(router.AddrPool) + for i, invoker := range invokers { + url := invoker.GetURL() + upsertBM(pool, "*", i) + tag := url.GetParam(constant.Tagkey, "") + upsertBM(pool, "tag\x00"+tag, i) + addr := url.Location + if addr != "" { + upsertBM(pool, "addr\x00"+addr, i) + if idx := strings.LastIndex(addr, ":"); idx > 0 { + upsertBM(pool, "port\x00"+addr[idx+1:], i) + } + } + for _, key := range []string{"version", "group"} { + if v := url.GetParam(key, ""); v != "" { + upsertBM(pool, "param\x00"+key+"\x00"+v, i) + } else { + upsertBM(pool, "param\x00"+key+"\x00\x00", i) + } + } + } + return pool, nil +} + +func (p *PriorityRouter) SetCache(cache router.Cache) { + p.cache = cache +} + +func (p *PriorityRouter) routeWithPool(invokers []base.Invoker, pool router.AddrPool, url *common.URL, invocation base.Invocation) []base.Invoker { + tag := invocation.GetAttachmentWithDefaultValue(constant.Tagkey, url.GetParam(constant.Tagkey, "")) + + application := invokers[0].GetURL().GetParam(constant.ApplicationKey, "") + key := strings.Join([]string{application, constant.TagRouterRuleSuffix}, "") + value, ok := p.routerConfigs.Load(key) + if !ok { + return collectInvokers(invokers, p.staticTagMatchBM(pool, tag, url, invocation)) + } + routerCfg := value.(global.RouterConfig) + enabled := routerCfg.Enabled == nil || *routerCfg.Enabled + valid := (routerCfg.Valid != nil && *routerCfg.Valid) || (routerCfg.Valid == nil && len(routerCfg.Tags) > 0) + if !enabled || !valid { + return collectInvokers(invokers, p.staticTagMatchBM(pool, tag, url, invocation)) + } + if tag == "" { + return collectInvokers(invokers, p.emptyTagMatchBM(pool, routerCfg)) + } + bm := p.requestTagMatchBM(pool, url, invocation, routerCfg, tag) + if bm == nil { + return requestTag(invokers, url, invocation, routerCfg, tag) + } + return collectInvokers(invokers, bm) +} + +func (p *PriorityRouter) staticTagMatchBM(pool router.AddrPool, tag string, url *common.URL, invocation base.Invocation) *roaring.Bitmap { + if tag != "" { + if bm := pool["tag\x00"+tag]; bm != nil && !bm.IsEmpty() { + return bm + } + if requestIsForce(url, invocation) { + return nil + } + } + return pool["tag\x00"] +} + +func (p *PriorityRouter) requestTagMatchBM(pool router.AddrPool, url *common.URL, invocation base.Invocation, + cfg global.RouterConfig, tag string) *roaring.Bitmap { + var ( + addresses []string + match []*common.ParamMatch + ) + for _, tagCfg := range cfg.Tags { + if tagCfg.Name == tag { + addresses = tagCfg.Addresses + match = tagCfg.Match + break + } + } + + var resultBM *roaring.Bitmap + if len(match) != 0 { + resultBM = p.matchBM(pool, match) + } else if len(addresses) != 0 { + resultBM = p.addressesBM(pool, addresses) + } else { + resultBM = pool["tag\x00"+tag] + } + + if (cfg.Force != nil && *cfg.Force) || requestIsForce(url, invocation) { + return resultBM + } + if resultBM != nil && !resultBM.IsEmpty() { + return resultBM + } + + emptyBM := pool["tag\x00"] + if emptyBM == nil { + return nil + } + if len(addresses) == 0 { + return emptyBM + } + addrBM := p.addressesBM(pool, addresses) + if addrBM != nil && !addrBM.IsEmpty() { + allBM := pool["*"] + if allBM != nil { + result := allBM.Clone() + result.AndNot(addrBM) + return result + } + } + return pool["*"] +} + +func (p *PriorityRouter) emptyTagMatchBM(pool router.AddrPool, cfg global.RouterConfig) *roaring.Bitmap { + bm := pool["tag\x00"] + if bm == nil || bm.IsEmpty() { + return bm + } + for _, tagCfg := range cfg.Tags { + if len(tagCfg.Addresses) == 0 { + continue + } + addrBM := p.addressesBM(pool, tagCfg.Addresses) + if addrBM != nil && !addrBM.IsEmpty() && bm != nil { + result := bm.Clone() + result.AndNot(addrBM) + bm = result + } + } + return bm +} + +func (p *PriorityRouter) addressesBM(pool router.AddrPool, addrs []string) *roaring.Bitmap { + bm := roaring.NewBitmap() + for _, addr := range addrs { + if ab := pool["addr\x00"+addr]; ab != nil { + bm.Or(ab) + } + if idx := strings.LastIndex(addr, ":"); idx > 0 && addr[:idx] == constant.AnyHostValue { + if pb := pool["port\x00"+addr[idx+1:]]; pb != nil { + bm.Or(pb) + } + } + } + return bm +} + +func (p *PriorityRouter) matchBM(pool router.AddrPool, matches []*common.ParamMatch) *roaring.Bitmap { + var result *roaring.Bitmap + for _, m := range matches { + bm := p.paramMatchBM(pool, m) + if bm == nil { + return nil + } + if result == nil { + result = bm.Clone() + } else { + result.And(bm) + } + if result.IsEmpty() { + return result + } + } + return result +} + +func (p *PriorityRouter) paramMatchBM(pool router.AddrPool, pm *common.ParamMatch) *roaring.Bitmap { + prefix := "param\x00" + pm.Key + "\x00" + switch { + case pm.Value.Exact != "": + return pool[prefix+pm.Value.Exact] + case pm.Value.Wildcard != "": + if pm.Value.Wildcard == constant.AnyValue { + return pool["*"] + } + return pool[prefix+pm.Value.Wildcard] + case pm.Value.Prefix != "": + return scanPrefixBM(pool, prefix, pm.Value.Prefix) + case pm.Value.Regex != "": + return scanRegexBM(pool, prefix, pm.Value.Regex) + case pm.Value.Empty != "": + return pool[prefix+"\x00"] + case pm.Value.Noempty != "": + emptyBM := pool[prefix+"\x00"] + allBM := pool["*"] + if allBM == nil { + return roaring.NewBitmap() + } + if emptyBM != nil { + result := allBM.Clone() + result.AndNot(emptyBM) + return result + } + return allBM + } + return roaring.NewBitmap() +} + +func collectInvokers(invokers []base.Invoker, bm *roaring.Bitmap) []base.Invoker { + if bm == nil || bm.IsEmpty() { + return []base.Invoker{} + } + result := make([]base.Invoker, 0, bm.GetCardinality()) + for _, idx := range bm.ToArray() { + if int(idx) < len(invokers) { + result = append(result, invokers[int(idx)]) + } + } + return result +} + +func upsertBM(pool router.AddrPool, key string, idx int) { + if _, ok := pool[key]; !ok { + pool[key] = roaring.NewBitmap() + } + pool[key].Add(uint32(idx)) +} + +func scanPrefixBM(pool router.AddrPool, prefix string, value string) *roaring.Bitmap { + bm := roaring.NewBitmap() + for key, entry := range pool { Review Comment: [P1] collectInvokers 使用 bm.ToArray() 将整个位图展开为 []uint32，再分配 []base.Invoker 切片，两次连续大分配增加 GC 压力。 建议：使用 `roaring.Bitmap.Iterator()` 逐个遍历，避免中间 []uint32 分配： ```go it := bm.Iterator() for it.HasNext() { idx := it.Next() // ... } ``` ########## cluster/router/tag/router.go: ########## @@ -157,3 +172,269 @@ func parseRoute(routeContent string) (*global.RouterConfig, error) { } return routerConfig, nil } + +func (p *PriorityRouter) Name() string { + return "tag" +} + +func (p *PriorityRouter) ShouldPool() bool { + return true +} + +func (p *PriorityRouter) Pool(invokers []base.Invoker) (router.AddrPool, router.AddrMetadata) { + pool := make(router.AddrPool) + for i, invoker := range invokers { + url := invoker.GetURL() + upsertBM(pool, "*", i) + tag := url.GetParam(constant.Tagkey, "") + upsertBM(pool, "tag\x00"+tag, i) + addr := url.Location + if addr != "" { + upsertBM(pool, "addr\x00"+addr, i) + if idx := strings.LastIndex(addr, ":"); idx > 0 { + upsertBM(pool, "port\x00"+addr[idx+1:], i) + } + } + for _, key := range []string{"version", "group"} { + if v := url.GetParam(key, ""); v != "" { + upsertBM(pool, "param\x00"+key+"\x00"+v, i) + } else { + upsertBM(pool, "param\x00"+key+"\x00\x00", i) + } + } + } + return pool, nil +} + +func (p *PriorityRouter) SetCache(cache router.Cache) { + p.cache = cache +} + +func (p *PriorityRouter) routeWithPool(invokers []base.Invoker, pool router.AddrPool, url *common.URL, invocation base.Invocation) []base.Invoker { + tag := invocation.GetAttachmentWithDefaultValue(constant.Tagkey, url.GetParam(constant.Tagkey, "")) + + application := invokers[0].GetURL().GetParam(constant.ApplicationKey, "") + key := strings.Join([]string{application, constant.TagRouterRuleSuffix}, "") + value, ok := p.routerConfigs.Load(key) + if !ok { + return collectInvokers(invokers, p.staticTagMatchBM(pool, tag, url, invocation)) + } + routerCfg := value.(global.RouterConfig) + enabled := routerCfg.Enabled == nil || *routerCfg.Enabled + valid := (routerCfg.Valid != nil && *routerCfg.Valid) || (routerCfg.Valid == nil && len(routerCfg.Tags) > 0) + if !enabled || !valid { + return collectInvokers(invokers, p.staticTagMatchBM(pool, tag, url, invocation)) + } + if tag == "" { + return collectInvokers(invokers, p.emptyTagMatchBM(pool, routerCfg)) + } + bm := p.requestTagMatchBM(pool, url, invocation, routerCfg, tag) + if bm == nil { + return requestTag(invokers, url, invocation, routerCfg, tag) + } + return collectInvokers(invokers, bm) +} + +func (p *PriorityRouter) staticTagMatchBM(pool router.AddrPool, tag string, url *common.URL, invocation base.Invocation) *roaring.Bitmap { + if tag != "" { + if bm := pool["tag\x00"+tag]; bm != nil && !bm.IsEmpty() { + return bm + } + if requestIsForce(url, invocation) { + return nil + } + } + return pool["tag\x00"] +} + +func (p *PriorityRouter) requestTagMatchBM(pool router.AddrPool, url *common.URL, invocation base.Invocation, + cfg global.RouterConfig, tag string) *roaring.Bitmap { + var ( + addresses []string + match []*common.ParamMatch + ) + for _, tagCfg := range cfg.Tags { + if tagCfg.Name == tag { + addresses = tagCfg.Addresses + match = tagCfg.Match + break + } + } + + var resultBM *roaring.Bitmap + if len(match) != 0 { + resultBM = p.matchBM(pool, match) + } else if len(addresses) != 0 { + resultBM = p.addressesBM(pool, addresses) + } else { + resultBM = pool["tag\x00"+tag] + } + + if (cfg.Force != nil && *cfg.Force) || requestIsForce(url, invocation) { + return resultBM + } + if resultBM != nil && !resultBM.IsEmpty() { + return resultBM + } + + emptyBM := pool["tag\x00"] + if emptyBM == nil { + return nil + } + if len(addresses) == 0 { + return emptyBM + } + addrBM := p.addressesBM(pool, addresses) + if addrBM != nil && !addrBM.IsEmpty() { + allBM := pool["*"] + if allBM != nil { + result := allBM.Clone() + result.AndNot(addrBM) + return result + } + } + return pool["*"] +} + +func (p *PriorityRouter) emptyTagMatchBM(pool router.AddrPool, cfg global.RouterConfig) *roaring.Bitmap { + bm := pool["tag\x00"] + if bm == nil || bm.IsEmpty() { + return bm + } + for _, tagCfg := range cfg.Tags { + if len(tagCfg.Addresses) == 0 { + continue + } + addrBM := p.addressesBM(pool, tagCfg.Addresses) + if addrBM != nil && !addrBM.IsEmpty() && bm != nil { + result := bm.Clone() + result.AndNot(addrBM) + bm = result + } + } + return bm +} + +func (p *PriorityRouter) addressesBM(pool router.AddrPool, addrs []string) *roaring.Bitmap { + bm := roaring.NewBitmap() + for _, addr := range addrs { + if ab := pool["addr\x00"+addr]; ab != nil { + bm.Or(ab) + } + if idx := strings.LastIndex(addr, ":"); idx > 0 && addr[:idx] == constant.AnyHostValue { + if pb := pool["port\x00"+addr[idx+1:]]; pb != nil { + bm.Or(pb) + } + } + } + return bm +} + +func (p *PriorityRouter) matchBM(pool router.AddrPool, matches []*common.ParamMatch) *roaring.Bitmap { + var result *roaring.Bitmap + for _, m := range matches { + bm := p.paramMatchBM(pool, m) + if bm == nil { + return nil + } + if result == nil { + result = bm.Clone() + } else { + result.And(bm) + } + if result.IsEmpty() { + return result + } + } + return result +} + +func (p *PriorityRouter) paramMatchBM(pool router.AddrPool, pm *common.ParamMatch) *roaring.Bitmap { + prefix := "param\x00" + pm.Key + "\x00" + switch { + case pm.Value.Exact != "": + return pool[prefix+pm.Value.Exact] + case pm.Value.Wildcard != "": + if pm.Value.Wildcard == constant.AnyValue { + return pool["*"] + } + return pool[prefix+pm.Value.Wildcard] + case pm.Value.Prefix != "": + return scanPrefixBM(pool, prefix, pm.Value.Prefix) + case pm.Value.Regex != "": + return scanRegexBM(pool, prefix, pm.Value.Regex) + case pm.Value.Empty != "": + return pool[prefix+"\x00"] + case pm.Value.Noempty != "": + emptyBM := pool[prefix+"\x00"] + allBM := pool["*"] + if allBM == nil { + return roaring.NewBitmap() + } + if emptyBM != nil { + result := allBM.Clone() + result.AndNot(emptyBM) + return result + } + return allBM + } + return roaring.NewBitmap() +} + +func collectInvokers(invokers []base.Invoker, bm *roaring.Bitmap) []base.Invoker { + if bm == nil || bm.IsEmpty() { + return []base.Invoker{} + } + result := make([]base.Invoker, 0, bm.GetCardinality()) + for _, idx := range bm.ToArray() { + if int(idx) < len(invokers) { + result = append(result, invokers[int(idx)]) + } + } + return result +} + +func upsertBM(pool router.AddrPool, key string, idx int) { + if _, ok := pool[key]; !ok { + pool[key] = roaring.NewBitmap() + } + pool[key].Add(uint32(idx)) +} + +func scanPrefixBM(pool router.AddrPool, prefix string, value string) *roaring.Bitmap { + bm := roaring.NewBitmap() + for key, entry := range pool { + if !strings.HasPrefix(key, prefix) { + continue + } + if strings.HasSuffix(key, "\x00") { + continue + } + val := key[len(prefix):] + if strings.HasPrefix(val, value) { + bm.Or(entry) + } + } + return bm +} + +func scanRegexBM(pool router.AddrPool, prefix string, pattern string) *roaring.Bitmap { + re, err := regexp.Compile(pattern) + if err != nil { + return roaring.NewBitmap() + } + bm := roaring.NewBitmap() + for key, entry := range pool { + if !strings.HasPrefix(key, prefix) { + continue Review Comment: [P1] scanRegexBM 每次调用都通过 regexp.Compile(pattern) 编译正则表达式。正则编译开销约 1-10μs，在热路径上是不必要的。 建议：在 Pool() 构建时预编译正则并缓存，或使用 sync.Pool 缓存编译结果。 ########## cluster/router/tag/router.go: ########## @@ -52,6 +57,16 @@ func (p *PriorityRouter) Route(invokers []base.Invoker, url *common.URL, invocat logger.Warnf("[tag router] invokers from previous router is empty") return invokers } + Review Comment: [P0] **核心语义缺陷**：缓存路径使用的 invokers 是全量快照 fullInvokers，忽略了路由链中前序路由器的过滤结果。当 finalInvokers 经过第一个路由器过滤后传入 TagRouter 时，routeWithPool 仍使用缓存中的全量列表和基于全量索引的位图，路由结果可能包含本应被前序路由排除的 invoker，破坏路由链正确性。 建议：在缓存路径中将传入的 invokers 与位图结果做交集，确保不包含已被前序路由排除的实例；或明确文档说明缓存仅在 TagRouter 是链中第一个路由时有效。 ########## cluster/router/chain/chain.go: ########## @@ -66,10 +67,13 @@ func (c *RouterChain) Route(url *common.URL, invocation base.Invocation) []base. if len(finalInvokers) == 0 { finalInvokers = invokers + } else if len(finalInvokers) != len(invokers) { + invocation.SetAttribute(constant.RouterCacheDisable, true) } Review Comment: [P0] RouterCacheDisable 仅在 `len(finalInvokers) != len(invokers)` 时设置，如果第一个路由器返回了相同数量但内容不同的 invokers（如替换了某些 invoker），则不会设置 RouterCacheDisable。若 TagRouter 恰好是第一个路由器，缓存会错误地被使用。 建议：在缓存查找时增加对传入 invokers 与缓存 invokers 引用一致性校验，或在 Route() 入口处重置 RouterCacheDisable 为 false。 ########## cluster/router/tag/router.go: ########## @@ -157,3 +172,269 @@ func parseRoute(routeContent string) (*global.RouterConfig, error) { } return routerConfig, nil Review Comment: [P1] routeWithPool 直接访问 invokers[0] 获取 ApplicationKey，如果 invokers 为空切片则会 panic。虽然调用方 Route() 已有空切片检查，但 routeWithPool 作为独立方法应做防御性检查：`if len(invokers) == 0 { return invokers }`。 ########## cluster/router/chain/cache.go: ########## @@ -0,0 +1,92 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package chain + +import ( + "sync" +) + +import ( + "dubbo.apache.org/dubbo-go/v3/cluster/router" + "dubbo.apache.org/dubbo-go/v3/protocol/base" +) + +// routerCache implements router.Cache by storing pre-computed AddrPool +// keyed by Poolable.Name(). It is rebuilt in its entirety on each +// SetInvokers call so that bitmap indices stay aligned with the current +// invoker snapshot. +type routerCache struct { + mu sync.RWMutex + invokers []base.Invoker + pools map[string]*poolEntry +} + +type poolEntry struct { + pool router.AddrPool +} + +func newRouterCache() *routerCache { + return &routerCache{ + pools: make(map[string]*poolEntry), + } +} + +func (c *routerCache) GetInvokers() []base.Invoker { + c.mu.RLock() + defer c.mu.RUnlock() + ret := make([]base.Invoker, len(c.invokers)) + copy(ret, c.invokers) + return ret +} + +func (c *routerCache) FindAddrPool(p router.Poolable) (router.AddrPool, []base.Invoker) { + c.mu.RLock() + defer c.mu.RUnlock() + entry, ok := c.pools[p.Name()] + if !ok { + return nil, nil + } Review Comment: [P2] FindAddrPool 每次调用 copy(invokers)，对于大量 invoker（N=1000+），这在热路径上产生频繁的内存分配。 建议：考虑使用不可变切片模式（invokers 切片一旦构建就不修改），直接返回引用而非复制。或使用 sync.Pool 复用切片缓冲区。 ########## cluster/router/tag/cache_benchmarks_test.go: ########## @@ -0,0 +1,216 @@ +/* Review Comment: [P2] 所有 benchmark 函数缺少 `b.ReportAllocs()` 调用，无法测量内存分配情况，对于缓存优化类 PR 至关重要。 建议：在每个 benchmark 函数开头添加 `b.ReportAllocs()`。 ########## cluster/router/tag/router.go: ########## @@ -157,3 +172,269 @@ func parseRoute(routeContent string) (*global.RouterConfig, error) { } return routerConfig, nil } + +func (p *PriorityRouter) Name() string { + return "tag" +} + +func (p *PriorityRouter) ShouldPool() bool { + return true +} + +func (p *PriorityRouter) Pool(invokers []base.Invoker) (router.AddrPool, router.AddrMetadata) { + pool := make(router.AddrPool) + for i, invoker := range invokers { Review Comment: [P2] 位图 key 使用 `\x00` 分隔符（如 `"tag\x00"`, `"addr\x00"`, `"param\x00"`），没有任何常量定义或文档说明。如果其他路由器也实现 Poolable，可能产生键冲突。 建议：定义键前缀常量（如 `const poolKeyTagPrefix = "tag\x00"`），并在 Pool() 方法注释中说明键空间划分规则。 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
