-
Notifications
You must be signed in to change notification settings - Fork 482
BuiltinValue: insertCoin and unionValue costing #7337
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
Closed
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
b893323 to
3c5708d
Compare
0d420fe to
75bd7a5
Compare
Extends the cost modeling framework to support lookupCoin, valueContains, valueData, and unValueData builtins. Adds parameter definitions, arity specifications, and integrates with the cost model generation system. Establishes foundation for accurate costing of Value operations in Plutus Core execution.
Creates Values.hs benchmark module with systematic test generation for lookupCoin, valueContains, valueData, and unValueData operations. Includes value generation utilities, individual benchmark functions, and edge case testing with empty values. Enables data collection for accurate cost model parameter fitting.
Implements optimal statistical models for Value operations based on performance characteristics: linear models for lookupCoin and valueContains (size-dependent), constant model for valueData (uniform performance), and linear model for unValueData. Provides accurate cost parameters across all builtin cost model configurations and updates test expectations.
Removes unimplementedCostingFun placeholders for Value builtins and connects them to their respective cost model parameters (paramLookupCoin, paramValueContains, paramValueData, paramUnValueData). Enables accurate execution cost calculation for Value operations in Plutus Core scripts.
Includes extensive benchmark results covering various input sizes and edge cases for lookupCoin, valueContains, valueData, and unValueData. Data validates the chosen statistical models and cost parameters. Provides empirical foundation confirming model accuracy across different operation profiles.
Add a new Logarithmic newtype wrapper in ExMemoryUsage that transforms size measures logarithmically. This enables linear cost models to effectively capture O(log n) runtime behavior by measuring log(size) instead of size directly. The wrapper computes max(1, floor(log2(size) + 1)) from any wrapped ExMemoryUsage instance, making it composable with existing size measures like ValueOuterOrMaxInner for operations with logarithmic complexity. This infrastructure supports proper costing of Value builtins like lookupCoin which has O(log max(m, k)) complexity.
Refactor the Value benchmarking suite to use Cardano-compliant key sizes (32-byte max) and leverage the new Logarithmic wrapper for accurate modeling of logarithmic operations. Key changes: - Apply Logarithmic wrapper to lookupCoin and valueContains benchmarks for proper O(log n) cost modeling - Consolidate key generators from 4 functions to 2, eliminating duplication - Remove obsolete key size parameters throughout (keys always maxKeyLen) - Extract withSearchKeys pattern to eliminate repetitive code - Simplify test generation by removing arbitrary key size variations - Clean up lookupCoinArgs structure for better readability The refactoring reduces the module from 359 to 298 lines while improving clarity and ensuring all generated Values comply with Cardano's 32-byte key length limit.
Simplify the R model definitions for Value-related builtins by replacing custom linear model implementation with standard linearInY wrapper for valueContains. This maintains the same statistical behavior while improving code maintainability. Add inline comments documenting the parameter wrapping strategy used for each model (Logarithmic wrapping for lookupCoin/valueContains, ValueTotalSize for contains operand, unwrapped for valueData/unValueData). Clean up formatting inconsistencies in model definitions.
Refreshed benchmarking data for lookupCoin, valueContains, valueData, and unValueData with improved statistical coverage and sampling. This data serves as the foundation for the refined cost model parameters applied in the subsequent commit.
Updated cost parameters based on fresh benchmark data analysis: - lookupCoin: Adjusted intercept (284421→179661) and slope (1→7151) to better reflect actual performance with varying currency counts - valueContains: Changed from added_sizes to linear_in_y model with refined parameters (intercept 42125119→1000, slope 30→130383) - valueData: Reduced constant cost (205465→153844) based on updated profiling results - unValueData: Switched to linear_in_x model with refined parameters (intercept 10532326261→1000, slope 431→33094) All three cost model variants (A, B, C) updated for consistency.
Modernize logarithm calculation in the Logarithmic ExMemoryUsage instance by switching from the compatibility module GHC.Integer.Logarithms to the modern GHC.Num.Integer API. Changes: - Replace integerLog2# (unboxed, from GHC.Integer.Logarithms) with integerLog2 (boxed, from GHC.Num.Integer) - Simplify code by removing unboxing boilerplate: I# (integerLog2# x) becomes integerLog2 x - Keep other imports (GHC.Integer.Logarithms, GHC.Exts) as they are still used elsewhere in the file (memoryUsageInteger function) This addresses code review feedback to use the modern ghc-bignum API instead of the legacy compatibility module, while maintaining the same computational semantics. Cost model regeneration verified no regression in derived parameters.
Address Kenneth's review comment by ensuring builtins use the same size measure wrappers as their budgeting benchmarks. Changes: - Add LogValueOuterOrMaxInner newtype combining logarithmic transformation with outer/max inner size measurement - Update lookupCoin and valueContains to use size measure wrappers - Add KnownTypeAst instances for ValueTotalSize and LogValueOuterOrMaxInner - Update benchmarks to use new combined wrapper type This ensures the cost model accurately reflects runtime behavior by using identical size measures in both denotations and benchmarks.
Regenerate cost model parameters based on fresh benchmark runs for the four Value-related built-in functions: lookupCoin, valueContains, valueData, and unValueData. New cost models: - lookupCoin: linear_in_z (intercept: 209937, slope: 7181) - valueContains: linear_in_y (intercept: 1000, slope: 131959) - valueData: constant_cost (182815) - unValueData: linear_in_x (intercept: 1000, slope: 33361) The benchmark data includes 350 measurement points across varying input sizes to ensure accurate cost estimation. All three cost model variants (A, B, C) have been updated consistently with identical parameters.
Document the regeneration of benchmark data and cost model parameters for the four Value-related built-in functions following fresh benchmark measurements.
…verhead Regenerate cost model parameters based on fresh benchmark runs after rebasing on master. This accounts for the negative amount validation added to valueContains in commit 531f1b8. Updated cost models: - lookupCoin: linear_in_z (intercept: 203599, slope: 7256) - valueContains: linear_in_y (intercept: 1000, slope: 130720) - valueData: constant_cost (156990) - unValueData: linear_in_x (intercept: 1000, slope: 36194) The benchmark data includes 350 measurement points across varying input sizes. All three cost model variants (A, B, C) have been updated consistently with identical parameters.
977bc39 to
bd8d31f
Compare
bd8d31f to
a018fc2
Compare
|
Closed in favor of #7384 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Needs #7344