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

feat: add batcher and com prep #204

Conversation

Itay-Tsabary-Starkware
Copy link
Contributor

@Itay-Tsabary-Starkware Itay-Tsabary-Starkware commented Jul 30, 2024

commit-id:96eb0ab9


This change is Reviewable

@Itay-Tsabary-Starkware Itay-Tsabary-Starkware force-pushed the pr/Itay-Tsabary-Starkware/tsabary/batcher/96eb0ab9 branch from 006c2af to 6427c60 Compare July 30, 2024 11:43
Copy link

Benchmark movements:
tree_computation_flow performance improved 😺
tree_computation_flow time: [34.153 ms 34.219 ms 34.295 ms]
change: [-4.2429% -2.6565% -1.1952%] (p = 0.00 < 0.05)
Performance has improved.
Found 5 outliers among 100 measurements (5.00%)
3 (3.00%) high mild
2 (2.00%) high severe

Copy link

codecov bot commented Jul 30, 2024

Codecov Report

Attention: Patch coverage is 0% with 29 lines in your changes missing coverage. Please review.

Project coverage is 76.82%. Comparing base (fe93a3b) to head (dd9ca10).
Report is 1 commits behind head on main.

Files Patch % Lines
crates/batcher/src/communication.rs 0.00% 29 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #204      +/-   ##
==========================================
- Coverage   76.89%   76.82%   -0.08%     
==========================================
  Files         316      317       +1     
  Lines       34504    34546      +42     
  Branches    34504    34546      +42     
==========================================
+ Hits        26532    26539       +7     
- Misses       5687     5722      +35     
  Partials     2285     2285              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@Itay-Tsabary-Starkware Itay-Tsabary-Starkware force-pushed the pr/Itay-Tsabary-Starkware/tsabary/batcher/96eb0ab9 branch from 6427c60 to b1a050b Compare July 31, 2024 10:05
@Itay-Tsabary-Starkware Itay-Tsabary-Starkware force-pushed the pr/Itay-Tsabary-Starkware/tsabary/batcher/aa33dbd8 branch 2 times, most recently from c6a5133 to ad45c95 Compare August 1, 2024 06:58
@Itay-Tsabary-Starkware Itay-Tsabary-Starkware force-pushed the pr/Itay-Tsabary-Starkware/tsabary/batcher/96eb0ab9 branch from b1a050b to 6228100 Compare August 1, 2024 06:58
Copy link

github-actions bot commented Aug 1, 2024

Benchmark movements:
tree_computation_flow performance regressed!
tree_computation_flow time: [34.099 ms 34.603 ms 35.193 ms]
change: [+2.0441% +3.3861% +5.4283%] (p = 0.00 < 0.05)
Performance has regressed.
Found 19 outliers among 100 measurements (19.00%)
5 (5.00%) high mild
14 (14.00%) high severe

@Itay-Tsabary-Starkware Itay-Tsabary-Starkware force-pushed the pr/Itay-Tsabary-Starkware/tsabary/batcher/96eb0ab9 branch from 6228100 to cae51bf Compare August 1, 2024 07:46
@Itay-Tsabary-Starkware Itay-Tsabary-Starkware force-pushed the pr/Itay-Tsabary-Starkware/tsabary/batcher/aa33dbd8 branch from ad45c95 to aee8b6b Compare August 1, 2024 07:46
Copy link

github-actions bot commented Aug 1, 2024

Benchmark movements:
tree_computation_flow performance regressed!
tree_computation_flow time: [34.664 ms 35.074 ms 35.525 ms]
change: [+1.0554% +2.9543% +4.7704%] (p = 0.00 < 0.05)
Performance has regressed.
Found 13 outliers among 100 measurements (13.00%)
7 (7.00%) high mild
6 (6.00%) high severe

Copy link

github-actions bot commented Aug 1, 2024

Benchmark movements:
tree_computation_flow performance regressed!
tree_computation_flow time: [34.662 ms 35.078 ms 35.550 ms]
change: [+1.1879% +2.5293% +3.9970%] (p = 0.00 < 0.05)
Performance has regressed.
Found 12 outliers among 100 measurements (12.00%)
5 (5.00%) high mild
7 (7.00%) high severe

@Itay-Tsabary-Starkware Itay-Tsabary-Starkware force-pushed the pr/Itay-Tsabary-Starkware/tsabary/batcher/96eb0ab9 branch from cae51bf to 051f29e Compare August 1, 2024 07:55
@Itay-Tsabary-Starkware Itay-Tsabary-Starkware force-pushed the pr/Itay-Tsabary-Starkware/tsabary/batcher/aa33dbd8 branch from aee8b6b to 75b2272 Compare August 1, 2024 07:55
@Itay-Tsabary-Starkware Itay-Tsabary-Starkware force-pushed the pr/Itay-Tsabary-Starkware/tsabary/batcher/96eb0ab9 branch from 051f29e to b92c64b Compare August 1, 2024 07:58
@Itay-Tsabary-Starkware Itay-Tsabary-Starkware force-pushed the pr/Itay-Tsabary-Starkware/tsabary/batcher/aa33dbd8 branch from 75b2272 to 5e9993f Compare August 1, 2024 07:58
Copy link

github-actions bot commented Aug 1, 2024

Benchmark movements:
tree_computation_flow performance improved 😺
tree_computation_flow time: [33.805 ms 33.953 ms 34.155 ms]
change: [-4.5418% -2.8909% -1.3923%] (p = 0.00 < 0.05)
Performance has improved.
Found 3 outliers among 100 measurements (3.00%)
1 (1.00%) high mild
2 (2.00%) high severe

@Itay-Tsabary-Starkware Itay-Tsabary-Starkware force-pushed the pr/Itay-Tsabary-Starkware/tsabary/batcher/96eb0ab9 branch from b92c64b to 2473411 Compare August 1, 2024 09:08
@Itay-Tsabary-Starkware Itay-Tsabary-Starkware force-pushed the pr/Itay-Tsabary-Starkware/tsabary/batcher/aa33dbd8 branch from 5e9993f to 503e4c9 Compare August 1, 2024 09:08
Copy link

github-actions bot commented Aug 1, 2024

Benchmark movements:
tree_computation_flow performance improved 😺
tree_computation_flow time: [33.791 ms 33.913 ms 34.057 ms]
change: [-4.1157% -2.5185% -1.0868%] (p = 0.00 < 0.05)
Performance has improved.
Found 13 outliers among 100 measurements (13.00%)
8 (8.00%) high mild
5 (5.00%) high severe

@Itay-Tsabary-Starkware Itay-Tsabary-Starkware force-pushed the pr/Itay-Tsabary-Starkware/tsabary/batcher/96eb0ab9 branch from 2473411 to b1de978 Compare August 4, 2024 12:56
@Itay-Tsabary-Starkware Itay-Tsabary-Starkware force-pushed the pr/Itay-Tsabary-Starkware/tsabary/batcher/aa33dbd8 branch 2 times, most recently from 561e1b2 to 7e8445b Compare August 4, 2024 13:58
@Itay-Tsabary-Starkware Itay-Tsabary-Starkware force-pushed the pr/Itay-Tsabary-Starkware/tsabary/batcher/96eb0ab9 branch from b1de978 to 6f774ba Compare August 4, 2024 13:58
Copy link

github-actions bot commented Aug 4, 2024

Benchmark movements:
tree_computation_flow performance improved 😺
tree_computation_flow time: [33.621 ms 33.707 ms 33.812 ms]
change: [-4.1576% -2.6262% -1.2781%] (p = 0.00 < 0.05)
Performance has improved.
Found 10 outliers among 100 measurements (10.00%)
2 (2.00%) high mild
8 (8.00%) high severe

Copy link

github-actions bot commented Aug 4, 2024

Benchmark movements:
full_committer_flow performance improved 😺
full_committer_flow time: [29.137 ms 29.191 ms 29.246 ms]
change: [-2.8459% -1.9923% -1.4025%] (p = 0.00 < 0.05)
Performance has improved.
Found 2 outliers among 100 measurements (2.00%)
2 (2.00%) high mild

@Itay-Tsabary-Starkware Itay-Tsabary-Starkware changed the base branch from pr/Itay-Tsabary-Starkware/tsabary/batcher/aa33dbd8 to main August 4, 2024 16:10
@Itay-Tsabary-Starkware Itay-Tsabary-Starkware force-pushed the pr/Itay-Tsabary-Starkware/tsabary/batcher/96eb0ab9 branch 2 times, most recently from 9105827 to d4c69b7 Compare August 4, 2024 16:15
Copy link
Collaborator

@dafnamatsry dafnamatsry left a comment

Choose a reason for hiding this comment

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

Reviewed 4 of 5 files at r1, 1 of 1 files at r2, 2 of 2 files at r3, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @Itay-Tsabary-Starkware)


crates/batcher/src/communication.rs line 16 at r1 (raw file):

use crate::batcher::Batcher;

pub type BatcherServer = LocalComponentServer<Batcher, BatcherRequest, BatcherResponse>;

Suggestion:

LocalBatcherServer

crates/batcher/src/communication.rs line 20 at r1 (raw file):

pub type RemoteBatcherServer = RemoteComponentServer<Batcher, BatcherRequest, BatcherResponse>;

pub fn create_batcher_server(

Suggestion:

create_local_batcher_server

@Itay-Tsabary-Starkware Itay-Tsabary-Starkware force-pushed the pr/Itay-Tsabary-Starkware/tsabary/batcher/96eb0ab9 branch from d4c69b7 to dd9ca10 Compare August 5, 2024 10:04
Copy link
Contributor Author

@Itay-Tsabary-Starkware Itay-Tsabary-Starkware left a comment

Choose a reason for hiding this comment

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

Reviewable status: 3 of 5 files reviewed, 2 unresolved discussions (waiting on @dafnamatsry)


crates/batcher/src/communication.rs line 16 at r1 (raw file):

use crate::batcher::Batcher;

pub type BatcherServer = LocalComponentServer<Batcher, BatcherRequest, BatcherResponse>;

Done.


crates/batcher/src/communication.rs line 20 at r1 (raw file):

pub type RemoteBatcherServer = RemoteComponentServer<Batcher, BatcherRequest, BatcherResponse>;

pub fn create_batcher_server(

Done.

Copy link
Collaborator

@dafnamatsry dafnamatsry left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewed 2 of 2 files at r4, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @Itay-Tsabary-Starkware)

@Itay-Tsabary-Starkware Itay-Tsabary-Starkware merged commit 3c7593d into main Aug 5, 2024
61 of 63 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Aug 7, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants