Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make hashcode and protobuf size cached, change model records to classes #344

Merged
merged 10 commits into from
Feb 14, 2025

Conversation

jasperpotts
Copy link
Member

PR to fix #343

@jasperpotts jasperpotts requested review from a team as code owners January 9, 2025 19:41
Copy link

github-actions bot commented Jan 9, 2025

JUnit Test Report

   67 files  ±0     67 suites  ±0   4m 48s ⏱️ + 1m 48s
1 265 tests ±0  1 262 ✅ ±0   3 💤 ±0  0 ❌ ±0 
7 120 runs  ±0  7 101 ✅ ±0  19 💤 ±0  0 ❌ ±0 

Results for commit 6525933. ± Comparison against base commit 381ac5b.

♻️ This comment has been updated with latest results.

Copy link

github-actions bot commented Jan 9, 2025

Integration Test Report

    383 files  ±0      383 suites  ±0   14m 29s ⏱️ - 1m 23s
114 358 tests ±0  114 358 ✅ ±0  0 💤 ±0  0 ❌ ±0 
114 586 runs  ±0  114 586 ✅ ±0  0 💤 ±0  0 ❌ ±0 

Results for commit 6525933. ± Comparison against base commit 381ac5b.

♻️ This comment has been updated with latest results.

@jasperpotts jasperpotts changed the title Eraly PR for Add compute on construction hashcode and protobuf size Early PR for Add compute on construction hashcode and protobuf size Jan 9, 2025
@jasperpotts jasperpotts requested review from a team as code owners January 9, 2025 20:33
Base automatically changed from 337-performance to main January 9, 2025 20:44
@jasperpotts jasperpotts force-pushed the 343-hash-size-caching branch from 4596db2 to 648f4b0 Compare January 9, 2025 21:41
@jasperpotts jasperpotts requested review from OlegMazurov and removed request for rbarker-dev January 9, 2025 21:43
@jasperpotts jasperpotts self-assigned this Jan 9, 2025
@jasperpotts jasperpotts force-pushed the 343-hash-size-caching branch from 7c9ec4c to 596bd1f Compare January 13, 2025 23:32
Signed-off-by: jasperpotts <[email protected]>
Signed-off-by: jasperpotts <[email protected]>
@jasperpotts jasperpotts changed the title Early PR for Add compute on construction hashcode and protobuf size Make hashcode and protobuf size cached, change model records to classes Jan 17, 2025
# Conflicts:
#	pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/Common.java
#	pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/generators/ModelGenerator.java
#	pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/generators/protobuf/CodecMeasureRecordMethodGenerator.java
#	pbj-core/pbj-runtime/src/main/java/com/hedera/pbj/runtime/RpcMethodDefinition.java
#	pbj-core/pbj-runtime/src/main/java/com/hedera/pbj/runtime/RpcServiceDefinition.java
#	pbj-integration-tests/src/jmh/java/com/hedera/pbj/intergration/jmh/JsonBench.java
#	pbj-integration-tests/src/jmh/java/com/hedera/pbj/intergration/jmh/ProtobufObjectBench.java
#	pbj-integration-tests/src/main/java/com/hedera/pbj/integration/NonSynchronizedByteArrayInputStream.java
Copy link
Member

@jsync-swirlds jsync-swirlds left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A couple suggestions to use printf instead of string concatenation in println.

Also noticed all of the generated code blocks now use tabs instead of spaces, which actually makes it pretty unreadable, particularly the larger code blocks which mix tabs and spaces a lot.

Signed-off-by: jasperpotts <[email protected]>
@jasperpotts jasperpotts merged commit cdb221f into main Feb 14, 2025
10 checks passed
@jasperpotts jasperpotts deleted the 343-hash-size-caching branch February 14, 2025 00:46
""".replace("$hashCodeManipulation", HASH_CODE_MANIPULATION).indent(DEFAULT_INDENT);
return $hashCode;
}
""".replace("$hashCodeManipulation", HASH_CODE_MANIPULATION)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should $hashCode be replaced with a field name, too?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nevermind. I see $hashCode is a real field name (which is confusing, btw)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add caching of hash code and protobuf size to model objects
4 participants