Repository: cloudwego/kitex
File: client/client.go (Go)

Code before:
	var ri rpcinfo.RPCInfo
	var callOpts *callopt.CallOptions
	ctx, ri, callOpts = kc.initRPCInfo(ctx, method)

	ctx = kc.opt.TracerCtl.DoStart(ctx, ri)
	var reportErr error
	var recycleRI bool
	defer func() {
		if panicInfo := recover(); panicInfo != nil {
			err = stats.ClientPanicToErr(ctx, panicInfo, ri, false)
			reportErr = err
		}
		kc.opt.TracerCtl.DoFinish(ctx, ri, reportErr)
		if recycleRI {
			// why need check recycleRI to decide if recycle RPCInfo?
			// 1. no retry, rpc timeout happen will cause panic when response return
			// 2. retry success, will cause panic when first call return
			// 3. backup request may cause panic, cannot recycle first RPCInfo
			// RPCInfo will be recycled after rpc is finished,
			// holding RPCInfo in a new goroutine is forbidden.
			rpcinfo.PutRPCInfo(ri)
		}
		callOpts.Recycle()
	}()

	callOptRetry, callOptFallback := initRetryAndFallbackWithCallopts(callOpts)
	if kc.opt.RetryContainer == nil && callOptRetry.Enable {
		// setup retry in callopt
		kc.opt.RetryContainer = retry.NewRetryContainer()
	}

	if kc.opt.RetryContainer == nil {
		// call without retry policy
		err = kc.eps(ctx, request, response)
		if err == nil {
			recycleRI = true
		}
	} else {
		recycleRI, err = kc.opt.RetryContainer.WithRetryIfNeeded(ctx, callOptRetry, kc.rpcCallWithRetry(ri, method, request, response), ri, request)
	}

	// do fallback if with setup
	err, reportErr = doFallbackIfNeeded(ctx, ri, request, response, err, callOptFallback, kc.opt.Fallback)
	return err
}

func (kc *kClient) rpcCallWithRetry(ri rpcinfo.RPCInfo, method string, request, response interface{}) retry.RPCCallFunc {
	// call with retry policy
	var callTimes int32
	// prevRI represents a value of rpcinfo.RPCInfo type.
	var prevRI atomic.Value

Review comment (suggestion):
callOptRetry is not a pointer to `retry.Policy`, and may be copied 5 times if enabled. Not sure whether it's be better to use a pointer since it's not very large (bool + int + 2 pointers).

Code after:
	var ri rpcinfo.RPCInfo
	var callOpts *callopt.CallOptions
	ctx, ri, callOpts = kc.initRPCInfo(ctx, method)

	ctx = kc.opt.TracerCtl.DoStart(ctx, ri)
	var reportErr error
	var recycleRI bool
	defer func() {
		if panicInfo := recover(); panicInfo != nil {
			err = stats.ClientPanicToErr(ctx, panicInfo, ri, false)
			reportErr = err
		}
		kc.opt.TracerCtl.DoFinish(ctx, ri, reportErr)
		if recycleRI {
			// why need check recycleRI to decide if recycle RPCInfo?
			// 1. no retry, rpc timeout happen will cause panic when response return
			// 2. retry success, will cause panic when first call return
			// 3. backup request may cause panic, cannot recycle first RPCInfo
			// RPCInfo will be recycled after rpc is finished,
			// holding RPCInfo in a new goroutine is forbidden.
			rpcinfo.PutRPCInfo(ri)
		}
		callOpts.Recycle()
	}()

	callOptRetry := getCalloptRetryPolicy(callOpts)
	if kc.opt.RetryContainer == nil && callOptRetry != nil && callOptRetry.Enable {
		// setup retry in callopt
		kc.opt.RetryContainer = retry.NewRetryContainer()
	}

	if kc.opt.RetryContainer == nil {
		// call without retry policy
		err = kc.eps(ctx, request, response)
		if err == nil {
			recycleRI = true
		}
	} else {
		recycleRI, err = kc.opt.RetryContainer.WithRetryIfNeeded(ctx, callOptRetry, kc.rpcCallWithRetry(ri, method, request, response), ri, request)
	}

	// do fallback if with setup
	err, reportErr = doFallbackIfNeeded(ctx, ri, request, response, err, kc.opt.Fallback, callOpts)
	return err
}

func (kc *kClient) rpcCallWithRetry(ri rpcinfo.RPCInfo, method string, request, response interface{}) retry.RPCCallFunc {
	// call with retry policy
	var callTimes int32
	// prevRI represents a value of rpcinfo.RPCInfo type.
	var prevRI atomic.Value