Simplify cost graphs and speedup costing performance #523
Conversation
aslesarenko
commented
Jun 10, 2019
•
aslesarenko
commented
- memory leak fixed in (resetContext is doing additional cleanups)
- removed dependency on scalan-meta.jar
- removed dependancy on scala-compiler.jar
- various performance optimisations (based on profiling)
- costing speedup by using max constants instead of variables (which simplifies cost graph and hence reduce time)
- decreased timeout in SpamSpeciafication
- fixed limit check in ValueSerializer
- MaxBoxSize set to 4 * 1024
- check MaxBoxSize and MaxPropositionBytes limits in deserialization
…position size on deserialization;
…-limit Check proposition size limit on deserialization
* move reader.positionLimit check from ValueSerializer to SigmaByteReader methods; * default SigmaByteReader.positionLimit to the whole remaining reader bytes; * extract max proposition size as a paramater in ErgoTreeSerializer.deserializeErgoTree; * honor maxTreeSize on constant deserialization in ErgoTreeSerializer.deserializeErgoTree; * remove checkPositionLimit in reader's getType and getValue; * add readers positionLimit setting in box deserialization; add restoration of readers positionLimit where it is altered; * restore serialization round trit check without prefixed random bytes;
# Conflicts: # src/test/scala/sigmastate/serialization/generators/ValueGenerators.scala
| val value = r.getULong() | ||
| val tree = ErgoTreeSerializer.DefaultSerializer.deserializeErgoTree(r) | ||
| val creationHeight = r.getUInt().toInt | ||
| val addTokensCount = r.getByte() |
There was a problem hiding this comment.
Looks like this breaks binary format?
There was a problem hiding this comment.
@catena2w I don't think so. We're writing unsigned values, but we were reading signed values. I just fixed reading.
There was a problem hiding this comment.
for small non-negative range they are the same @inline override def getUByte(): Int = getByte() & 0xFF
| @@ -127,15 +128,18 @@ class ErgoTreeSerializer { | |||
| deserializeErgoTree(r) | |||
| } | |||
|
|
|||
| def deserializeErgoTree(r: SigmaByteReader): ErgoTree = { | |||
| def deserializeErgoTree(r: SigmaByteReader, | |||
| maxTreeSizeBytes: Int = SigmaSerializer.MaxPropositionSize): ErgoTree = { | |||
There was a problem hiding this comment.
Is this really a function parameter? Let's either remove it from parameters list or remove the default value
There was a problem hiding this comment.
@catena2w Probably not. Easing of testing does not count. Apart from that, decoupling from SigmaSerializer constant does not worth it alone. Removing default value will just force this constant leaking out. Let me remove the parameter.
There was a problem hiding this comment.
removed default value.
| cost(s"{ $rootTree.updateDigest($rootTree.digest) == $rootTree }") shouldBe | ||
| (AccessRootHash + selectField + newAvlTreeCost + comparisonCost /* for isConstantSize AvlTree type */) | ||
| cost(s"{ $rootTree.updateOperations(1.toByte) == $rootTree }") shouldBe | ||
| (AccessRootHash + newAvlTreeCost + comparisonCost + constCost) |
There was a problem hiding this comment.
Let's add tests for changed costing rules, they should be quite easy now.
There was a problem hiding this comment.
only this tests are affected and changed. Others are not affected.
More test will be added in #516
…tate-interpreter into simplify-costing
