Skip to content

Commit

Permalink
addressing comments/updates
Browse files Browse the repository at this point in the history
  • Loading branch information
@igor-aptos
igor-aptos committed Oct 17, 2023
1 parent bb9bc64 commit 65b233d
Show file tree
Hide file tree
Showing 3 changed files with 143 additions and 85 deletions.
59 changes: 45 additions & 14 deletions aptos-move/framework/aptos-framework/doc/aggregator_v2.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,8 @@ across multiple transactions, enabling parallel execution. For example, if the
first transaction is doing <code><a href="aggregator_v2.md#0x1_aggregator_v2_try_add">try_add</a>(X, 1)</code> for aggregator <code>X</code>, and the second is
doing <code><a href="aggregator_v2.md#0x1_aggregator_v2_try_sub">try_sub</a>(X,3)</code>, they can be executed in parallel avoiding a read-modify-write
dependency.
However, reading the aggregator value (i.e. calling <code><a href="aggregator_v2.md#0x1_aggregator_v2_read">read</a>(X)</code>) is an expensive
operation and should be avoided as much as possible because it reduces the
parallelism.
However, reading the aggregator value (i.e. calling <code><a href="aggregator_v2.md#0x1_aggregator_v2_read">read</a>(X)</code>) is a resource-intensive
operation that also reduced parallelism, and should be avoided as much as possible.


- [Struct `Aggregator`](#0x1_aggregator_v2_Aggregator)
Expand All @@ -32,6 +31,7 @@ parallelism.
- [Function `copy_snapshot`](#0x1_aggregator_v2_copy_snapshot)
- [Function `read_snapshot`](#0x1_aggregator_v2_read_snapshot)
- [Function `string_concat`](#0x1_aggregator_v2_string_concat)
- [Function `test_aggregator_valid_type`](#0x1_aggregator_v2_test_aggregator_valid_type)
- [Specification](#@Specification_1)
- [Function `create_aggregator`](#@Specification_1_create_aggregator)
- [Function `try_add`](#@Specification_1_try_add)
Expand Down Expand Up @@ -59,7 +59,7 @@ across multiple transactions. See the module description for more details.
Currently supported types for Element are u64 and u128.


<pre><code><b>struct</b> <a href="aggregator_v2.md#0x1_aggregator_v2_Aggregator">Aggregator</a>&lt;IntElement&gt; <b>has</b> store
<pre><code><b>struct</b> <a href="aggregator_v2.md#0x1_aggregator_v2_Aggregator">Aggregator</a>&lt;IntElement&gt; <b>has</b> drop, store
</code></pre>


Expand Down Expand Up @@ -141,23 +141,23 @@ The value of aggregator underflows (goes below zero). Raised by uncoditional sub



<a name="0x1_aggregator_v2_EAGGREGATOR_FUNCTION_NOT_YET_SUPPORTED"></a>
<a name="0x1_aggregator_v2_EAGGREGATOR_API_V2_NOT_ENABLED"></a>

The native aggregator function, that is in the move file, is not yet supported.
and any calls will raise this error.
The aggregator api v2 feature flag is not enabled.


<pre><code><b>const</b> <a href="aggregator_v2.md#0x1_aggregator_v2_EAGGREGATOR_FUNCTION_NOT_YET_SUPPORTED">EAGGREGATOR_FUNCTION_NOT_YET_SUPPORTED</a>: u64 = 9;
<pre><code><b>const</b> <a href="aggregator_v2.md#0x1_aggregator_v2_EAGGREGATOR_API_V2_NOT_ENABLED">EAGGREGATOR_API_V2_NOT_ENABLED</a>: u64 = 6;
</code></pre>



<a name="0x1_aggregator_v2_EAGGREGATOR_SNAPSHOTS_NOT_ENABLED"></a>
<a name="0x1_aggregator_v2_EAGGREGATOR_FUNCTION_NOT_YET_SUPPORTED"></a>

The aggregator snapshots feature flag is not enabled.
The native aggregator function, that is in the move file, is not yet supported.
and any calls will raise this error.


<pre><code><b>const</b> <a href="aggregator_v2.md#0x1_aggregator_v2_EAGGREGATOR_SNAPSHOTS_NOT_ENABLED">EAGGREGATOR_SNAPSHOTS_NOT_ENABLED</a>: u64 = 6;
<pre><code><b>const</b> <a href="aggregator_v2.md#0x1_aggregator_v2_EAGGREGATOR_FUNCTION_NOT_YET_SUPPORTED">EAGGREGATOR_FUNCTION_NOT_YET_SUPPORTED</a>: u64 = 9;
</code></pre>


Expand Down Expand Up @@ -371,7 +371,9 @@ If subtraction would result in a negative value, <code><b>false</b></code> is re
## Function `read`

Returns a value stored in this aggregator.
Note: This operation prevents parallelism of the transaction that calls it.
Note: This operation is resource-intensive, and reduces parallelism.
(Especially if called in a transaction that also modifies the aggregator,
or has other read/write conflicts)


<pre><code><b>public</b> <b>fun</b> <a href="aggregator_v2.md#0x1_aggregator_v2_read">read</a>&lt;IntElement&gt;(<a href="aggregator.md#0x1_aggregator">aggregator</a>: &<a href="aggregator_v2.md#0x1_aggregator_v2_Aggregator">aggregator_v2::Aggregator</a>&lt;IntElement&gt;): IntElement
Expand All @@ -395,7 +397,7 @@ Note: This operation prevents parallelism of the transaction that calls it.
## Function `snapshot`

Returns a wrapper of a current value of an aggregator
Unlike read(), this enables parallel execution of transactions.
Unlike read(), it is fast and avoids sequential dependencies.


<pre><code><b>public</b> <b>fun</b> <a href="aggregator_v2.md#0x1_aggregator_v2_snapshot">snapshot</a>&lt;IntElement&gt;(<a href="aggregator.md#0x1_aggregator">aggregator</a>: &<a href="aggregator_v2.md#0x1_aggregator_v2_Aggregator">aggregator_v2::Aggregator</a>&lt;IntElement&gt;): <a href="aggregator_v2.md#0x1_aggregator_v2_AggregatorSnapshot">aggregator_v2::AggregatorSnapshot</a>&lt;IntElement&gt;
Expand Down Expand Up @@ -466,7 +468,9 @@ NOT YET IMPLEMENTED, always raises EAGGREGATOR_FUNCTION_NOT_YET_SUPPORTED.
## Function `read_snapshot`

Returns a value stored in this snapshot.
Note: This operation prevents parallelism of the transaction that calls it.
Note: This operation is resource-intensive, and reduces parallelism.
(Especially if called in a transaction that also modifies the aggregator,
or has other read/write conflicts)


<pre><code><b>public</b> <b>fun</b> <a href="aggregator_v2.md#0x1_aggregator_v2_read_snapshot">read_snapshot</a>&lt;Element&gt;(snapshot: &<a href="aggregator_v2.md#0x1_aggregator_v2_AggregatorSnapshot">aggregator_v2::AggregatorSnapshot</a>&lt;Element&gt;): Element
Expand Down Expand Up @@ -509,6 +513,33 @@ If length of prefix and suffix together exceed 256 bytes, ECONCAT_STRING_LENGTH_



</details>

<a name="0x1_aggregator_v2_test_aggregator_valid_type"></a>

## Function `test_aggregator_valid_type`



<pre><code><b>public</b> <b>fun</b> <a href="aggregator_v2.md#0x1_aggregator_v2_test_aggregator_valid_type">test_aggregator_valid_type</a>()
</code></pre>



<details>
<summary>Implementation</summary>


<pre><code><b>public</b> <b>fun</b> <a href="aggregator_v2.md#0x1_aggregator_v2_test_aggregator_valid_type">test_aggregator_valid_type</a>() {
<a href="aggregator_v2.md#0x1_aggregator_v2_create_unbounded_aggregator">create_unbounded_aggregator</a>&lt;u64&gt;();
<a href="aggregator_v2.md#0x1_aggregator_v2_create_unbounded_aggregator">create_unbounded_aggregator</a>&lt;u128&gt;();
<a href="aggregator_v2.md#0x1_aggregator_v2_create_aggregator">create_aggregator</a>&lt;u64&gt;(5);
<a href="aggregator_v2.md#0x1_aggregator_v2_create_aggregator">create_aggregator</a>&lt;u128&gt;(5);
}
</code></pre>



</details>

<a name="@Specification_1"></a>
Expand Down
126 changes: 71 additions & 55 deletions aptos-move/framework/aptos-framework/sources/aggregator_v2/aggregator_v2.move
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,8 @@
/// first transaction is doing `try_add(X, 1)` for aggregator `X`, and the second is
/// doing `try_sub(X,3)`, they can be executed in parallel avoiding a read-modify-write
/// dependency.
/// However, reading the aggregator value (i.e. calling `read(X)`) is an expensive
/// operation and should be avoided as much as possible because it reduces the
/// parallelism.
/// However, reading the aggregator value (i.e. calling `read(X)`) is a resource-intensive
/// operation that also reduced parallelism, and should be avoided as much as possible.
module aptos_framework::aggregator_v2 {
use std::error;
use std::string::String;
Expand All @@ -22,8 +21,8 @@ module aptos_framework::aggregator_v2 {
/// The generic type supplied to the aggregator snapshot is not supported.
const EUNSUPPORTED_AGGREGATOR_SNAPSHOT_TYPE: u64 = 5;

/// The aggregator snapshots feature flag is not enabled.
const EAGGREGATOR_SNAPSHOTS_NOT_ENABLED: u64 = 6;
/// The aggregator api v2 feature flag is not enabled.
const EAGGREGATOR_API_V2_NOT_ENABLED: u64 = 6;

/// The generic type supplied to the aggregator is not supported.
const EUNSUPPORTED_AGGREGATOR_TYPE: u64 = 7;
Expand All @@ -39,7 +38,7 @@ module aptos_framework::aggregator_v2 {
/// across multiple transactions. See the module description for more details.
///
/// Currently supported types for Element are u64 and u128.
struct Aggregator<IntElement> has store {
struct Aggregator<IntElement> has store, drop {
value: IntElement,
max_value: IntElement,
}
Expand Down Expand Up @@ -90,11 +89,13 @@ module aptos_framework::aggregator_v2 {
}

/// Returns a value stored in this aggregator.
/// Note: This operation prevents parallelism of the transaction that calls it.
/// Note: This operation is resource-intensive, and reduces parallelism.
/// (Especially if called in a transaction that also modifies the aggregator,
/// or has other read/write conflicts)
public native fun read<IntElement>(aggregator: &Aggregator<IntElement>): IntElement;

/// Returns a wrapper of a current value of an aggregator
/// Unlike read(), this enables parallel execution of transactions.
/// Unlike read(), it is fast and avoids sequential dependencies.
public native fun snapshot<IntElement>(aggregator: &Aggregator<IntElement>): AggregatorSnapshot<IntElement>;

/// Creates a snapshot of a given value.
Expand All @@ -105,7 +106,9 @@ module aptos_framework::aggregator_v2 {
public native fun copy_snapshot<Element: copy + drop>(snapshot: &AggregatorSnapshot<Element>): AggregatorSnapshot<Element>;

/// Returns a value stored in this snapshot.
/// Note: This operation prevents parallelism of the transaction that calls it.
/// Note: This operation is resource-intensive, and reduces parallelism.
/// (Especially if called in a transaction that also modifies the aggregator,
/// or has other read/write conflicts)
public native fun read_snapshot<Element>(snapshot: &AggregatorSnapshot<Element>): Element;

/// Concatenates `before`, `snapshot` and `after` into a single string.
Expand All @@ -114,85 +117,98 @@ module aptos_framework::aggregator_v2 {
/// If length of prefix and suffix together exceed 256 bytes, ECONCAT_STRING_LENGTH_TOO_LARGE is raised.
public native fun string_concat<IntElement>(before: String, snapshot: &AggregatorSnapshot<IntElement>, after: String): AggregatorSnapshot<String>;

#[test(_fx = @std)]
public fun test_correct_read(_fx: &signer) {
// use std::features;
// let feature = features::get_aggregator_v2_api_feature();
// features::change_feature_flags(fx, vector[feature], vector[]);
#[test]
public fun test_aggregator() {
let agg = create_aggregator(10);
assert!(try_add(&mut agg, 5), 1);
assert!(try_add(&mut agg, 5), 2);
assert!(read(&agg) == 10, 3);
assert!(!try_add(&mut agg, 5), 4);
assert!(read(&agg) == 10, 5);
assert!(try_sub(&mut agg, 5), 6);
assert!(read(&agg) == 5, 7);

let snap = snapshot(&agg);
assert!(try_add(&mut agg, 2), 8);
assert!(read(&agg) == 7, 9);
assert!(read_snapshot(&snap) == 5, 10);
}

#[test]
public fun test_correct_read() {
let snapshot = create_snapshot(42);
// copy not yet supported
// let snapshot2 = copy_snapshot(&snapshot);
assert!(read_snapshot(&snapshot) == 42, 0);
// assert!(read_snapshot(&snapshot2) == 42, 0);
}

#[test(_fx = @std)]
public fun test_correct_read_string(_fx: &signer) {
// use std::features;
// let feature = features::get_aggregator_v2_api_feature();
// features::change_feature_flags(fx, vector[feature], vector[]);

let snapshot = create_snapshot(std::string::utf8(b"42"));
// copy not yet supported
// let snapshot2 = copy_snapshot(&snapshot);
assert!(read_snapshot(&snapshot) == std::string::utf8(b"42"), 0);
// assert!(read_snapshot(&snapshot2) == std::string::utf8(b"42"), 0);
}

#[test(_fx = @std)]
public fun test_string_concat1(_fx: &signer) {
// use std::features;
// let feature = features::get_aggregator_v2_api_feature();
// features::change_feature_flags(fx, vector[feature], vector[]);
#[test]
#[expected_failure(abort_code = 0x030009, location = Self)]
public fun test_copy_not_yet_supported() {
let snapshot = create_snapshot(42);
copy_snapshot(&snapshot);
}

#[test]
public fun test_string_concat1() {
let snapshot = create_snapshot(42);
let snapshot2 = string_concat(std::string::utf8(b"before"), &snapshot, std::string::utf8(b"after"));
assert!(read_snapshot(&snapshot2) == std::string::utf8(b"before42after"), 0);
}

#[test(_fx = @std)]
#[test]
#[expected_failure(abort_code = 0x030005, location = Self)]
public fun test_string_concat2(_fx: &signer) {
// use std::features;
// let feature = features::get_aggregator_v2_api_feature();
// features::change_feature_flags(fx, vector[feature], vector[]);

public fun test_string_concat_from_string_not_supported() {
let snapshot = create_snapshot<String>(std::string::utf8(b"42"));
let snapshot2 = string_concat(std::string::utf8(b"before"), &snapshot, std::string::utf8(b"after"));
read_snapshot(&snapshot2);
// String concatenation not supported
// assert!(read_snapshot(&snapshot2) == std::string::utf8(b"before42after"), 0);
string_concat(std::string::utf8(b"before"), &snapshot, std::string::utf8(b"after"));
}

// #[test(fx = @std)]
// #[expected_failure(abort_code = 0x030006, location = Self)]
// public fun test_snapshot_feature_not_enabled(fx: &signer) {
// use std::features;
// use aptos_framework::reconfiguration;
// let feature = features::get_aggregator_v2_api_feature();
// features::change_feature_flags(fx, vector[], vector[feature]);

// reconfiguration::reconfigure_for_test();
// create_snapshot(42);
// }

#[test(_fx = @std)]
// #[test(fx = @std)]
// #[expected_failure(abort_code = 0x030006, location = Self)]
// public fun test_aggregator_feature_not_enabled(fx: &signer) {
// use std::features;
// use aptos_framework::reconfiguration;
// let feature = features::get_aggregator_v2_api_feature();
// features::change_feature_flags(fx, vector[], vector[feature]);
// reconfiguration::reconfigure_for_test();
// create_aggregator(42);
// }

#[test]
#[expected_failure(abort_code = 0x030007, location = Self)]
public fun test_aggregator_invalid_type1() {
create_unbounded_aggregator<u8>();
}

public fun test_aggregator_valid_type() {
create_unbounded_aggregator<u64>();
create_unbounded_aggregator<u128>();
create_aggregator<u64>(5);
create_aggregator<u128>(5);
}

#[test]
#[expected_failure(abort_code = 0x030005, location = Self)]
public fun test_snpashot_invalid_type1(_fx: &signer) {
// use std::features;
public fun test_snpashot_invalid_type1() {
use std::option;
// let feature = features::get_aggregator_v2_api_feature();
// features::change_feature_flags(fx, vector[feature], vector[]);

create_snapshot(option::some(42));
}

#[test(_fx = @std)]
#[test]
#[expected_failure(abort_code = 0x030005, location = Self)]
public fun test_snpashot_invalid_type2(_fx: &signer) {
// use std::features;
// let feature = features::get_aggregator_v2_api_feature();
// features::change_feature_flags(fx, vector[feature], vector[]);

public fun test_snpashot_invalid_type2() {
create_snapshot(vector[42]);
}
}
Loading

0 comments on commit 65b233d

Please sign in to comment.