Repository navigation
Conversation
🛠 PR Checks SummaryAll Automated Checks passed. ✅ Manual Checks (for Reviewers):
Read More🤖 This bot helps streamline PR reviews by verifying automated checks and providing guidance for contributors and reviewers. ✅ Automated Checks (for Contributors):🟢 Maintainers must be able to edit this pull request (more info) ☑️ Contributor Actions:
☑️ Reviewer Actions:
📚 Resources:Debug
|
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
| OpCPUReturnCallDefers = 78 | ||
|
|
||
| /* Print */ | ||
| OpCPUPrinln = 1 |
There was a problem hiding this comment.
Nit: "OpCPUPrintln" not "OpCPUPrinln"
|
Maybe to close in favor of #3921. |
Yes. i added you as a reviewer. |
notJoon
left a comment
There was a problem hiding this comment.
LGTM
remove: review/triage-pending flag
| /* Print */ | ||
| OpCPUPrintln = 1 | ||
| OpCPUPrint = 1 |
There was a problem hiding this comment.
Can you get this information from benchops, rather than just assuming 1?
There was a problem hiding this comment.
Do you know how to calculate the related gas with this ?
There was a problem hiding this comment.
like benchmark other OpCodes to calculate this.
however i'm good with a "guess value" here within this PR. Since there are some other places with the same issue, ie GC.
Eventually, we need a comprehensive benchmark after to fulfill all of these.
|
Closing in favour of #3921, apologies |
| fmt.Fprintf(builder, " %s(%d) %s\n", gens, gen, pv.PkgPath) | ||
| } else { | ||
| bsi := b.StringIndented(" ") | ||
| bsi := b.StringIndented(NewStringBuilderWithGasMeter(m.GasMeter), " ") |
There was a problem hiding this comment.
why use metering only in some cases here and not replace the builder type with a metered builder?
There was a problem hiding this comment.
yes right we can use same builder
ltzmaxwell
left a comment
There was a problem hiding this comment.
get start reading and thinking
| WriteString(s string) (int, error) | ||
| WriteByte(c byte) error | ||
| String() string | ||
| } |
There was a problem hiding this comment.
is this necessary? at least it should not be here in values.go
| type Value interface { | ||
| assertValue() | ||
| String() string // for debugging | ||
| WriteString(builder Builder) Builder |
| "github.com/gnolang/gno/tm2/pkg/store" | ||
| ) | ||
|
|
||
| type StringBuilderWithGasMeter struct { |
There was a problem hiding this comment.
| type StringBuilderWithGasMeter struct { | |
| type MeteredStringBuilder struct { |
| strings.Builder | ||
| } | ||
|
|
||
| func NewStringBuilderWithGasMeter(gasMeter store.GasMeter) *StringBuilderWithGasMeter { |
There was a problem hiding this comment.
| func NewStringBuilderWithGasMeter(gasMeter store.GasMeter) *StringBuilderWithGasMeter { | |
| func NewMeteredBuilder(gasMeter store.GasMeter) *MeteredStringBuilder{ |
|
there are several PRs all related to gas consumption: for value stringer, for allocation, for frameSize? etc. will come back later and read these together. |
Yes, part of this PR will be fixed by #4546. I will remove the gas calculation during stringification and focus on the gas CPU consumption caused by printing. |
|
Replaced by #4705 |
|
Replaced by #4705 |
alternative to #3949 **Native Benchmark** ``` op,avg_time,avg_size,time_stddev,count NativePrint_1,3742,0,761,5999 NativePrint_1000,4267,0,4088,5997 NativePrint_10000,5617,0,239,5995 ``` **OP Code Benchmark** ``` op,avg_time,avg_size,time_stddev,count OpAdd,48,0,61,1999 OpAddAssign,141,0,229,1000 OpArrayLit,237,0,616,5001 OpArrayType,154,0,62,7 OpAssign,156,0,285,4001 OpBand,49,0,54,1000 OpBandAssign,95,0,60,1000 OpBandn,50,0,178,1000 OpBandnAssign,100,0,294,1000 OpBinary1,52,0,129,3002 OpBody,103,0,349,18001 OpBor,47,0,44,1000 OpBorAssign,89,0,57,1000 OpCall,649,0,5239,17999 OpCallDeferNativeBody,77,0,60,1001 OpCallNativeBody,323,0,1046,5000 OpChanType,104,0,21,2 OpCompositeLit,105,0,614,7000 OpConvert,111,0,284,1001 OpDec,89,0,88,2001 OpDefer,292,0,6233,3001 OpDefine,197,0,904,14999 OpEql,105,0,118,1999 OpEval,64,0,253,18321 OpExec,45,0,62,18002 OpFieldType,155,0,71,39 OpForLoop,59,0,57,2000 OpFuncLit,282,0,1265,3001 OpFuncType,183,0,1022,3056 OpGeq,49,0,47,1000 OpGtr,63,0,225,2000 OpHalt,21,0,70,36514 OpIfCond,94,0,142,1999 OpInc,106,0,188,3000 OpIndex1,114,0,241,3001 OpIndex2,189,0,341,1000 OpInterfaceType,241,0,274,46 OpLand,59,0,178,2001 OpLeq,52,0,77,1000 OpLor,67,0,122,1000 OpLss,45,0,77,2999 OpMapLit,742,0,2287,1000 OpMapType,131,0,261,1006 OpMul,52,0,103,1000 OpMulAssign,99,0,145,1000 OpNeq,56,0,21,1000 OpPanic2,44,0,54,1000 OpPopBlock,30,0,57,21496 OpPopFrameAndReset,40,0,54,2000 OpPopResults,23,0,41,4000 OpPopValue,26,0,34,2999 OpPrecall,267,0,899,18000 OpQuo,143,0,246,1000 OpQuoAssign,213,0,345,1000 OpRangeIter,236,0,636,2000 OpRangeIterArrayPtr,57,0,59,1000 OpRangeIterMap,67,0,115,1000 OpRangeIterString,91,0,139,1000 OpRef,239,0,859,2000 OpRem,137,0,425,1000 OpRemAssign,170,0,252,1000 OpReturn,68,0,106,5000 OpReturnCallDefers,375,0,1182,3001 OpReturnFromBlock,76,0,119,17999 OpReturnToBlock,68,0,210,1001 OpSelector,103,0,203,1000 OpShl,63,0,330,1000 OpShlAssign,111,0,98,1000 OpShr,59,0,124,1000 OpShrAssign,108,0,180,1000 OpSlice,249,0,874,1000 OpSliceLit,443,0,1529,2000 OpSliceLit2,612,0,1396,1000 OpSliceType,172,0,104,15 OpStar,117,0,194,2000 OpStructLit,371,0,977,1999 OpStructType,361,0,226,3 OpSub,46,0,60,1000 OpSubAssign,119,0,183,1000 OpSwitchClause,91,0,418,1000 OpSwitchClauseCase,98,0,109,1000 OpTypeAssert1,618,0,577,1000 OpTypeAssert2,421,0,440,1000 OpTypeDecl,147,0,90,1003 OpTypeSwitch,91,0,88,1000 OpUneg,59,0,88,1000 OpUnot,34,0,64,1000 OpUpos,26,0,62,1001 OpUxor,43,0,17,1000 OpValueDecl,166,0,257,4001 OpVoid,26,0,59,36515 OpXor,52,0,48,1000 OpXorAssign,105,0,188,1000 ``` --------- Co-authored-by: ltzmaxwell <ltz.maxwell@gmail.com>
closes: #3819