Windowed SCP for reads and writes #117

mundya · 2015-04-27T10:22:39Z

Uses sliding window protocol to increase speed for bursts of SCP messages.

As of 14th May this results in reads and writes to chip (0, 0) of around 30+Mbit/s when using a window size of 8. When using a window size of 1 this drops to (a still respectable) 12ish Mbit/s on my test machine.

Neaten documentation
Rework send_scp to send a burst of one message (mostly to
reduce duplication of code)
Fill in a few gaps in the tests.
Set the default window size to 1
~~Correctly query the window size from SC&MP~~ For a later PR
~~Set the transient IPTag timeout correctly~~ For a later PR

mundya · 2015-04-27T10:25:05Z

Looks like this will be plagued by timeouts...

mundya · 2015-04-27T10:25:53Z

rig/machine_control/machine_controller.py

Default should be 1, should actually query the machine to determine what this value is.

**Supercedes #115** Uses sliding window protocol to increase speed for bursts of SCP messages. Talking to a locally connection SpiNN3 I'm hitting 20+ Mb/s for both writes and reads to (chip 0, 0). - [x] Neaten documentation - [x] Rework `send_scp` to send a burst of one message (mostly to reduce duplication of code) - [x] Fill in a few gaps in the tests. - [ ] Set the default window size to 1 - [ ] Correctly query the window size from SC&MP - [ ] Set the transient IPTag timeout correctly

mundya · 2015-04-27T16:11:31Z

rig/machine_control/scp_connection.py

Default mask can, I think, be safely 0xffff (and should be).

mundya · 2015-05-13T20:03:07Z

rig/machine_control/scp_connection.py

unpack_from would make sense here.

Further increases SCP performance on reads and writes by not unpacking SCP packets unless absolutely necessary.

mundya · 2015-05-13T21:18:32Z

With tonight's changes this is pushing nearer to 27+Mib/s on 10MiB reads and writes to a locally connected Spin3 board.

mundya · 2015-05-13T21:30:35Z

rig/machine_control/scp_connection.py

This is not good... (the padding bytes are already included in outstanding.packet) and probably explains the frequent time outs. This is also a bug in the tests.

Previously the padding bytes were being included twice in retransmitted packets. This fixes that bug and introduces a test to ensure that retransmitted packets are transmitted exactly.

mundya · 2015-05-14T06:29:32Z

Cancelling running tests is a bad thing that I shouldn't do...

mossblaser · 2015-05-14T06:33:08Z

Is it a bad thing?

mundya · 2015-05-14T06:39:09Z

With the window size set to 1 this branch gets reads and writes at around 11-12Mbit/s against my local board (0, 0) and 5Mbit/s against (1, 1). (Master is at around 8-9Mbit/s and 4Mbit/s.)

@mossblaser do we want to consider merging this now with the window size set to 1? We can investigate setting the TTO and reading the window size in an additional PR.

mossblaser · 2015-05-14T06:41:20Z

Via a switch on my machine, window size = 1 I get:

Finished write after 11422 ms. Throughput = 7.00 Mbit/s
Finished read after 11520 ms. Throughput = 6.94 Mbit/s

Just upping the window now...

mossblaser · 2015-05-14T06:42:46Z

..and with window size 8:

Finished write after 2441 ms. Throughput = 32.78 Mbit/s
Finished read after 2404 ms. Throughput = 33.28 Mbit/s

Very good!

mundya · 2015-05-14T06:43:46Z

Yay C-like performance :)

mossblaser · 2015-05-14T06:44:19Z

If you want a laugh... here's the C numbers:

Writing 10485760 bytes of random data to 0x60240000...
Write completed in 2463 ms! Throughput = 32.481 Mbit/s.

Reading back 10485760 bytes from 0x60240000...
Read completed in 2619 ms! Throughput = 30.546 Mbit/s.

...so you beat rig-scp ;) :P

mundya · 2015-05-14T06:45:07Z

PyPy time :P

mossblaser · 2015-05-14T06:51:50Z

@mossblaser do we want to consider merging this now with the window size set to 1? We can investigate setting the TTO and reading the window size in an additional PR.

I would say that seems like a good idea. Will have a look over the code as it stands first though :)

mundya · 2015-05-14T06:52:32Z

Will have a look over the code as it stands first though :)

Of course!

mossblaser · 2015-05-14T06:58:44Z

rig/machine_control/scp_connection.py

Would a closure not be sufficient?

No, because the received packet has to be stored somewhere.

In Python 3 I can do:

packet = None def callback(received_bytes): nonlocal packet packet = ...

But not in Python 2.

mossblaser · 2015-05-14T07:12:13Z

rig/machine_control/scp_connection.py

Doesn't the format string at the start not imply the length here too?

Yes. Why? The constant is the offset.

mossblaser · 2015-05-14T07:21:07Z

Looks jolly good to me, though that excludes the busy loop(!) which should be an easy enough fix and drop the CPU loading down a fair bit. Plus would save a load of "get time" system calls (so actual performance may be ever-so-slightly improved... but that is a dubious claim ;)).

mundya · 2015-05-14T07:33:21Z

rig/machine_control/scp_connection.py

Noted by @mossblaser: this should be a while

Unused argument.

@mossblaser

- Reduces the number of calls to create a bytestring - Implements @mossblaser suggestion of calling `iter` on `parameters_and_callbacks`.

mundya · 2015-05-14T09:08:11Z

I'm finding that select, and switching to using a while instead of an if make for a fairly significant performance hit. You're welcome to play.

mundya · 2015-05-14T09:13:31Z

Also, that 100% is not 100%... there's a branch missing in scp_connection.py.

Interprets some error codes (RC_P2P_xxx) as if they were a timeout. With a modest window size this allows correct behaviour when talking to anything but an ethernet connected chip.

mundya · 2015-05-14T14:29:31Z

With the exception of select, which I graciously leave open for @mossblaser to do, I'm happy that we consider this for merging soon. Tasks for SpiNNaker 1.4 are to investigate querying the correct window size and setting the TTO.

This bizarrely introduces some overhead which, despite numerous attempts to track down its source, has not entirely been eliminated. That said, it does bring the CPU load *way* down. Strangely, this patch negatively impacts write performance slightly (for both 0,0 and 1,1) and read performance for 0,0 but positively impacts read performance for 1,1. Further experimentation is required...

@mossblaser

@mossblaser, I've fixed a broken test. There are still failing tests for you to investigate.

mundya · 2015-05-29T17:24:09Z

@mossblaser, if I merge in your select work and get the tests to pass again can we consider merging this soon?

- Mocks out "select" so that it will work with mocks. - Changes tests to deal with slight change in send/receive operation.

`min` doesn't have a keyword `default` earlier than Python 3.4, this removes the requirement and adds a test that tests the newly added branch works correctly.

mossblaser · 2015-05-30T13:04:49Z

That sounds like a plan :) thanks!

Jonathan
On 29 May 2015 18:24, "Andrew Mundy" notifications@github.com wrote:

@mossblaser https://github.com/mossblaser, if I merge in your select
work and get the tests to pass again can we consider merging this soon?

—
Reply to this email directly or view it on GitHub
#117 (comment).

Windowed SCP with `select`

mundya · 2015-05-31T14:19:43Z

I'm not sure what's up with Coveralls... coverage remains at 100% though. @mossblaser, are we happy with this?

mossblaser · 2015-06-01T09:45:51Z

Yep I'm happy. Silly Coveralls... Merge and release at will! Sorry about the delay ;)

Windowed SCP for reads and writes

mundya · 2015-06-02T10:41:34Z

Thanks

mundya added enhancement work in progress labels Apr 27, 2015

mundya reviewed Apr 27, 2015
View reviewed changes

mundya force-pushed the even-faster-scp-rw2 branch from 3ebcdaf to e183fce Compare April 27, 2015 16:09

mundya reviewed Apr 27, 2015
View reviewed changes

rig/machine_control/scp_connection.py Outdated

Copy link

Member Author

mundya Apr 27, 2015

Choose a reason for hiding this comment

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

Default mask can, I think, be safely 0xffff (and should be).

mossblaser mentioned this pull request Apr 29, 2015

(Optional) Buffered writing via file-IO. #124

Closed

mundya added this to the SpiNNaker Tools 1.4 milestone Apr 29, 2015

mundya reviewed May 13, 2015
View reviewed changes

rig/machine_control/scp_connection.py Outdated

Copy link

Member Author

mundya May 13, 2015

Choose a reason for hiding this comment

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

unpack_from would make sense here.

mundya added 3 commits May 13, 2015 21:11

Merge branch 'master' into even-faster-scp-rw2

ee51a49

Reduce the amount of unpacking.

dad37ad

Further increases SCP performance on reads and writes by not unpacking SCP packets unless absolutely necessary.

Safely set the default window size.

62ad6be

mundya reviewed May 13, 2015
View reviewed changes

mundya added 2 commits May 14, 2015 07:13

Use the full range of packet seq values.

915698e

Fix retransmission of packets.

f25c795

Previously the padding bytes were being included twice in retransmitted packets. This fixes that bug and introduces a test to ensure that retransmitted packets are transmitted exactly.

mundya force-pushed the even-faster-scp-rw2 branch from 4d83ffd to f25c795 Compare May 14, 2015 06:21

mossblaser reviewed May 14, 2015
View reviewed changes

mundya reviewed May 14, 2015
View reviewed changes

rig/machine_control/scp_connection.py Outdated

Copy link

Member Author

mundya May 14, 2015

Choose a reason for hiding this comment

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

Noted by @mossblaser: this should be a while

mossblaser mentioned this pull request May 14, 2015

Spot all SCP Errors #141

Closed

mundya added 3 commits May 14, 2015 09:17

Remove expected_args from scp_call.

f9a07c1

Unused argument.

Reduce calls to bytestring, allow iterables.

00db8a5

- Reduces the number of calls to create a bytestring - Implements @mossblaser suggestion of calling `iter` on `parameters_and_callbacks`.

Reduce number of timeout calculations.

2c3fd04

Fix missing error codes.

c151b26

Interprets some error codes (RC_P2P_xxx) as if they were a timeout. With a modest window size this allows correct behaviour when talking to anything but an ethernet connected chip.

mossblaser and others added 2 commits May 15, 2015 13:48

Fix infinite test.

3f3e1b5

@mossblaser, I've fixed a broken test. There are still failing tests for you to investigate.

mundya added 2 commits May 29, 2015 21:27

Fix tests for scp_connection.

8fc268b

- Mocks out "select" so that it will work with mocks. - Changes tests to deal with slight change in send/receive operation.

flake8 and spelling

d289c99

mundya mentioned this pull request May 29, 2015

Windowed SCP with select #152

Merged

Fix Python 2.7 support

3de4c9a

`min` doesn't have a keyword `default` earlier than Python 3.4, this removes the requirement and adds a test that tests the newly added branch works correctly.

Merge pull request #152 from project-rig/even-faster-scp-rw2-select

f1c61af

Windowed SCP with `select`

mossblaser added a commit that referenced this pull request Jun 2, 2015

Merge pull request #117 from project-rig/even-faster-scp-rw2

fdfb28c

Windowed SCP for reads and writes

mossblaser merged commit fdfb28c into master Jun 2, 2015

mossblaser deleted the even-faster-scp-rw2 branch June 2, 2015 07:09

mossblaser mentioned this pull request Jun 2, 2015

Another stab at high performance I/O #77

Closed

Windowed SCP for reads and writes #117

Windowed SCP for reads and writes #117

Uh oh!

Conversation

mundya commented Apr 27, 2015

Uh oh!

mundya commented Apr 27, 2015

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mundya commented May 13, 2015

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mundya commented May 14, 2015

Uh oh!

mossblaser commented May 14, 2015

Uh oh!

mundya commented May 14, 2015

Uh oh!

mossblaser commented May 14, 2015

Uh oh!

mossblaser commented May 14, 2015

Uh oh!

mundya commented May 14, 2015

Uh oh!

mossblaser commented May 14, 2015

Uh oh!

mundya commented May 14, 2015

Uh oh!

mossblaser commented May 14, 2015

Uh oh!

mundya commented May 14, 2015

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mossblaser commented May 14, 2015

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mundya commented May 14, 2015

Uh oh!

mundya commented May 14, 2015

Uh oh!

mundya commented May 14, 2015

Uh oh!

mundya commented May 29, 2015

Uh oh!

mossblaser commented May 30, 2015

Uh oh!

mundya commented May 31, 2015

Uh oh!

mossblaser commented Jun 1, 2015

Uh oh!

mundya commented Jun 2, 2015

Uh oh!

Uh oh!