Skip to content

Conversation

marcpiechura
Copy link
Contributor

closes #2263

@alexvaluyskiy
Copy link
Contributor

This performance test is failed - Akka.Streams.Tests.Performance.GraphBuilderBenchmark+Graph_with_1000_imported_flows
[FAIL] Expected Elapsed Time to must be less than or equal to 70.00 ms; actual value was 72.67 ms.

Also I've found a lot of similar exceptions in performance tests, like this

 Invoking setup for Akka.Streams.Tests.Performance.FlowSelectBenchmark+Flow_Select_100k_elements_with_buffer_8_and_1_select_and_without_GraphStage_Identifier
[00:56:45][Step 2/2] ERROR: Error occurred during $Akka.Streams.Tests.Performance.FlowSelectBenchmark+Flow_Select_100k_elements_with_buffer_8_and_1_select_and_without_GraphStage_Identifier SETUP.
[00:56:45][Step 2/2] System.ArgumentNullException: Value cannot be null.
[00:56:45][Step 2/2] Parameter name: stream
[00:56:45][Step 2/2]    at System.IO.StreamReader..ctor(Stream stream, Encoding encoding, Boolean detectEncodingFromByteOrderMarks, Int32 bufferSize, Boolean leaveOpen)
[00:56:45][Step 2/2]    at System.IO.StreamReader..ctor(Stream stream)
[00:56:45][Step 2/2]    at Akka.Configuration.ConfigurationFactory.FromResource(String resourceName, Assembly assembly) in D:\BuildAgent\work\d26c9d7f36545acd\src\core\Akka\Configuration\ConfigurationFactory.cs:line 135
[00:56:45][Step 2/2]    at Akka.Streams.Tests.Performance.FlowSelectBenchmark.Setup(BenchmarkContext context) in D:\BuildAgent\work\d26c9d7f36545acd\src\core\Akka.Streams.Tests.Performance\FlowSelectBenchmark.cs:line 56
[00:56:45][Step 2/2]    at NBench.Sdk.ReflectionBenchmarkInvoker.InvokePerfSetup(BenchmarkContext context)
[00:56:45][Step 2/2]    at NBench.Sdk.Benchmark.PreRun()

@alexvaluyskiy
Copy link
Contributor

alexvaluyskiy commented Sep 5, 2016

@Silv3rcircl3 you should check this test StreamLayout_should_not_fail_materialization_when_building_a_large_graph_with_simple_computation_when_using_Via

Xunit can't execute it and just fails. I've checked it 3 times

@@ -88,14 +92,16 @@ public void A_Graph_with_materialized_value_must_expose_the_materialized_value_a
return new SourceShape<Task<int>>(b.MaterializedValue);
}));

[Fact]
public void A_Graph_with_materialized_value_must_allow_exposing_the_materialized_value_as_port()
[InlineData(true, false)]
Copy link
Contributor

Choose a reason for hiding this comment

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

Probably you should use

[InlineData(true)]
[InlineData(false)]

@marcpiechura
Copy link
Contributor Author

@alexvaluyskiy addressed your remarks

@alexvaluyskiy
Copy link
Contributor

LGTM

@Aaronontheweb
Copy link
Member

just needs a rebase

@marcpiechura
Copy link
Contributor Author

@Aaronontheweb rebased but @Horusiath wanted to take a look too so he should merge it once he has reviewed it.

@@ -82,7 +81,7 @@ public override IModule WithAttributes(Attributes attributes)
protected override SinkModule<NotUsed, Task<Attributes>> NewInstance(SinkShape<NotUsed> shape)
=> new AttributesSink(Attributes, shape);

public override ISubscriber<NotUsed> Create(MaterializationContext context, out Task<Attributes> materializer)
public override object Create(MaterializationContext context, out Task<Attributes> materializer)
Copy link
Contributor

Choose a reason for hiding this comment

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

Why object instead of ISubscriber?

Copy link
Contributor Author

@marcpiechura marcpiechura Sep 6, 2016

Choose a reason for hiding this comment

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

@Horusiath
Copy link
Contributor

This is enormous amount of work, but guys - that performance drop seriously worries me (and no, setting perf threshold to be more forgiving is not a way to go). From what I know, performance in JVM never drops in higher versions, and I think we should not let this happen on our side.

This is strange, as i.e. Keep optimization and forgetting materialized values should make it faster.

@marcpiechura
Copy link
Contributor Author

marcpiechura commented Sep 6, 2016

The perf specs passed yesterday and today failed with this ~50% perf drop without any code changes. So I guess it has something to do with the build server.

@marcpiechura
Copy link
Contributor Author

marcpiechura commented Sep 6, 2016

For example the Graph_with_1000_junction failed in the last run
[FAIL] Expected Elapsed Time to must be less than or equal to 1,000.00 ms; actual value was 1,422.67 ms.

but passed the last 3 runs before
[PASS] Expected Elapsed Time to must be less than or equal to 1,000.00 ms; actual value was 808.33 ms.
[PASS] Expected Elapsed Time to must be less than or equal to 1,000.00 ms; actual value was 787.67 ms.
[PASS] Expected Elapsed Time to must be less than or equal to 1,000.00 ms; actual value was 771.33 ms.

@Aaronontheweb Aaronontheweb merged commit ade2176 into akkadotnet:dev Sep 6, 2016
@Aaronontheweb Aaronontheweb added this to the 1.1.2 milestone Sep 6, 2016
@marcpiechura marcpiechura deleted the streams_update_2.4.3 branch May 13, 2017 19:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Couldn't build an actor materializer settings. akka.stream.materializer config path is not defined.
4 participants