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

Dart: update benchmark to Dart 3.0 #364

Closed
wants to merge 17 commits into from

Conversation

amka
Copy link

@amka amka commented Jul 3, 2023

Summary of changes
Changes introduced in this pull request:

  • Update Dart SDK to 3.0
  • Remove gRPC package deprecations
  • Fix Docker build stage

Reference issue to close (if applicable)
Closes #146

As fart as I think Dart is required for this benchmark 'cause it's one of the "official" grpc languages I fixed the benchmark. Current version of Dart is 3.0.5 so the SDK was updated to this release. Also dependencies and the pubspec file were updated accordingly.
I also renamed benchmark folder to be able to activate this benchmark. Testes locally and it worked like a charm.

Hope it help! Any commend are appreciated!

dart_grpc_bench/Dockerfile Outdated Show resolved Hide resolved
@LesnyRumcajs
Copy link
Owner

@amka Thanks! Could you please update the CI? You can either call build.sh (or bench.sh) with the Dart suite or run ./generate_ci.sh >.github/workflows/build.yml and commit that.

@amka
Copy link
Author

amka commented Jul 3, 2023

Sure, I will do it!

@LesnyRumcajs
Copy link
Owner

The diff is a bit off, like the CI worker is sorting the benchmarks differently. I'll have a look later.

@LesnyRumcajs
Copy link
Owner

I'm a bit pushed for time; I'll try to check this out this weekend @amka!

@amka
Copy link
Author

amka commented Jul 20, 2023

I'm a bit pushed for time; I'll try to check this out this weekend @amka!

Oh, no worries! It's your repo! I'm just the guy who's trying to help :)

@LesnyRumcajs
Copy link
Owner

Not sure why the script generated it for you this way, let's not bother with it. If you'd put dart after d to make CI happy it should be fine. Oh, and if you could please do these changes with git mv that'd be great, the old dart_grpc_onhold is still lingering there.

@amka
Copy link
Author

amka commented Sep 18, 2023

Not sure why the script generated it for you this way, let's not bother with it. If you'd put dart after d to make CI happy it should be fine. Oh, and if you could please do these changes with git mv that'd be great, the old dart_grpc_onhold is still lingering there.

@LesnyRumcajs Not sure I understand you correctly about git mv. Do you mean to replace the old one _onhold with a new _bench folder?

@LesnyRumcajs
Copy link
Owner

Not sure why the script generated it for you this way, let's not bother with it. If you'd put dart after d to make CI happy it should be fine. Oh, and if you could please do these changes with git mv that'd be great, the old dart_grpc_onhold is still lingering there.

@LesnyRumcajs Not sure I understand you correctly about git mv. Do you mean to replace the old one _onhold with a new _bench folder?

Yes, precisely. The _onhold suffix means that the particular bench suite was disabled for whatever reason. Your contribution effectively replaces it. At the current state of the PR, you can just remove it.

@LesnyRumcajs
Copy link
Owner

@amka did you try running it locally recently? It seems to be failing in the CI.

@amka
Copy link
Author

amka commented Sep 19, 2023

I've got a weird issue. When I ran sh ./bench.sh dart_grpc_bench locally for the first time it worked like a charm. When I rerun it - it fails quickly. We'll investigate it.

@LesnyRumcajs
Copy link
Owner

LesnyRumcajs commented Feb 9, 2024

@amka I fixed the meta check (whitespace issue), now the dart server is failing. I debugged it a bit, locally. The server seems to be crashing from time to time (most of the times it works just fine) during the warmup period. See the error message:

❯ docker logs dart_grpc_bench -f
Server listening on port 50051...
Unhandled exception:
Null check operator used on a null value
#0      ServerHandler.sendTrailers (package:grpc/src/server/handler.dart:384)
#1      ServerHandler._sendError (package:grpc/src/server/handler.dart:459)
#2      ServerHandler._onResponse (package:grpc/src/server/handler.dart:327)
#3      _RootZone.runUnaryGuarded (dart:async/zone.dart:1594)
#4      _BufferingStreamSubscription._sendData (dart:async/stream_impl.dart:339)
#5      _BufferingStreamSubscription._add (dart:async/stream_impl.dart:271)
#6      _SyncStreamControllerDispatch._sendData (dart:async/stream_controller.dart:784)
#7      _StreamController._add (dart:async/stream_controller.dart:658)
#8      new Stream.fromFuture.<anonymous closure> (dart:async/stream.dart:250)
#9      Future._propagateToListeners.handleValueCallback (dart:async/future_impl.dart:846)
#10     Future._propagateToListeners (dart:async/future_impl.dart:875)
#11     Future._chainCoreFutureSync (dart:async/future_impl.dart:575)
#12     Future._chainCoreFutureAsync.<anonymous closure> (dart:async/future_impl.dart:613)
#13     _microtaskLoop (dart:async/schedule_microtask.dart:40)
#14     _startMicrotaskLoop (dart:async/schedule_microtask.dart:49)
#15     _runPendingImmediateCallback (dart:isolate-patch/isolate_patch.dart:118)
#16     _RawReceivePort._handleMessage (dart:isolate-patch/isolate_patch.dart:185)

It might make sense to bump the dependencies, perhaps the ones defined in pubspec.yaml and pubspec.lock are faulty?

@LesnyRumcajs
Copy link
Owner

Stale. Closing.

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.

Re-enable Dart benchmark
3 participants