-
Notifications
You must be signed in to change notification settings - Fork 67
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
Implemented Arranging-Streams | Don't MERGE #145
Conversation
I think I'm more inclined to call them |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a good start. I'm curious to see how the integration will work.
/// # Remarks | ||
/// - Iterator mutates the `expected_index`. | ||
/// - You can't use this iterator for iterating trough all cached values. | ||
pub fn iter_mut(&mut self) -> IterMut<T> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should probably have a more descriptive name than IterMut
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't agree, 'iter' an 'iter_mut' are standeralized names in rust, checkout the rust collections. Take note that the type returned here implements an iterator.
The user can call '.next()' on this iterator as long there are pack the available who are in order
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was specifically referring to the type name. I guess it is common in rust to have an IterMut
struct in the namespace representing the mutable iterator so I guess it's fine (even if I don't love it).
@jstnlef I get your thoughts, but there are a couple of reason why I didn't go for
First, before I came up with the term 'arranging', I called it 'sorting' and before that 'ordering'. This is about describing a process that puts items, based on their id, in order. This process is a continuous process and is not executed at a certain moment in time. Because sorting is about a single operation I left that term. I could indeed have chosen 'ordering', ordering is, according to the dictionary: "this is the process of putting something in a particular order". Only there was a problem with this, there were two instances of this process, 1) sequencing, 2) ordering. Because 'ordering' was already used to describe an instance of this process, I did not want to associate this process with 'ordering'. There are three things we have to keep apart here:
I don't want to assemble 1 and 3 so you don't get confused by terminology. An example of this is 'ordering streams', here we could talk about the 'stream' that orders the packets (3) or we could talk about the abstract process (1) that describes how we handle packets. Because of this, I had started looking for new terminology to better describe this and so I came up with 'arranging'. According to the dictionaries, this means: " To put into a specific order or relation; dispose: arrange shoes in a neat row". 1 or " to put a group of objects in a particular order" 2. I understand in the first respect why you don't find sequencing fitting 'arranging' here. But still, if I compare it with the definition, the 'arranging of items so that we only get the newer ones' seems like it falls under the definition.
I think
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I took a quick read through this, I think it overall looks good, the name arranging
I'm not a huge fan of just personally sounds kinda awkward and out of place. Besides that, for the data structures, what do you mean writes are slow? Hashmaps should have very very fast writes.
/// Returns the number of streams currently created. | ||
fn stream_count(&self) -> usize; | ||
/// Try to get a `Stream` by `stream_id`. When the stream does not exist, it will be inserted by the given `stream_id` and returned. | ||
fn get_or_create_stream(&mut self, stream_id: u8) -> &mut Self::Stream; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You probably want to move the generated stream out of here instead of dealing with returning a mut ref which might cause complicated lifetime issues for implementors of this trait.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not sure what you mean here? move out?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are returning a reference instead of moving the stream out of the function.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Alright, I get that, but why would we want to move it out? We shouldn't remove the stream from the HashMap and return it back to the user. It must stay in the Hashmap as long packets are queued on it.
I explained why I choose for that name, however, if you guys like to see another name here could you provide alternatives? Since I think it is a well-fitting name and I am not sure about any alternatives. |
@LucioFranco The cost of writing and/or executing the hashing-function could be high if the requirement for collision avoidance is strict, or if you have a small hash-space, that's why I said that. |
@TimonPost is it high enough to slow down laminar? I wouldn't think that would be the case. |
Cool, fortunately :) Also, there would not be that much written, so we will be fine. |
Are there any problems with merging this? I will remove it from the mod.rs so that it does not affect clippy with unused code. If this is merged it will be easier to create the next PR implementing this. I already started on that. |
_"Sequencing: this is the process of only caring about the newest items."_ [1](https://dictionary.cambridge.org/dictionary/english/sequencing) _"Ordering: this is the process of putting something in a particular order."_ [2](https://dictionary.cambridge.org/dictionary/english/ordering) - Sequencing: Only the newest items will be passed trough e.g. `1,3,2,5,4` which results in `1,3,5`. - Ordering: All items are returned in order `1,3,2,5,4` which results in `1,2,3,4,5`. What are these 'arranging streams'? You can see 'arranging streams' as something to arrange items that have no relationship at all with one another. Think of a highway where you have several lanes where cars are driving. Because there are these lanes, cars can move on faster. For example, the cargo drivers drive on the right and the high-speed cars on the left. The cargo drivers have no influence on fast cars and vice versa. If a game developer wants to send data to a client, it may be that he wants to send data ordered, unordered or sequenced. Data might be the following: 1. Player movement, we want to order player movements because we don't care about old positions. 2. Bullet movement, we want to order bullet movement because we don't care about old positions of bullets. 3. Chat messages, we want to order chat messages because it is nice to see the text in the right order. Player movement and chat messages are totally unrelated to each other and you absolutely do not want that movement packets are interrupted when a chat message is not sent. With ordering, we can only return items when all packets up to the current package are received. So if a chat package is missing, the other packages will suffer from it. It would be nice if we could order player movements and chat messages separately. This is exactly what `ordering streams` are meant for. The game developer can indicate on which stream he can order his packets and how he wants to arrange them. For example, the game developer can say: "Let me set all chat messages to 'stream 1' and all motion packets to 'stream 2'.
PR-GOAL
What it is about
This PR is about arranging items, over different streams, based on a certain algorithm.
The above sentence contains a lot of important information, we take this apart in the up-coming document.
What it is not about
This PR does not implement the code for the actual ordering/sequencing of our packets, like checking the packet type and based on that order or sequence it. This only implements the code which is responsible for ordering or sequencing and managing some state around that. This PR should not get merged, it is only meant for reviewing, and for preventing one big PR.
Before this PR should be merged I want to have a good design idea for #143.
Other
See notices for discussions and proposals of this PR.
I hosted the rust docs here, which will give you a better perspective than reading the rust doc comments self.
Table of Contents
Ordering VS Sequencing
Let's define two concepts here:
"Sequencing: this is the process of only caring about the newest items." 1
"Ordering: this is the process of putting something in a particular order." 2
1,3,2,5,4
which results in1,3,5
.1,3,2,5,4
which results in1,2,3,4,5
.Arranging Streams
What are these 'arranging streams'?
You can see 'arranging streams' as something to arrange items that have no relationship at all with one another.
Simple Example
Think of a highway where you have several lanes where cars are driving.
Because there are these lanes, cars can move on faster.
For example, the cargo drivers drive on the right and the high-speed cars on the left.
The cargo drivers have no influence on fast cars and vice versa.
Real Example
If a game developer wants to send data to a client, it may be that he wants to send data ordered, unordered or sequenced.
Data might be the following:
Player movement and chat messages are totally unrelated to each other and you absolutely do not want that movement packets are interrupted when a chat message is not sent.
With ordering, we can only return items when all packets up to the current package are received.
So if a chat package is missing, the other packages will suffer from it.
It would be nice if we could order player movements and chat messages separately. This is exactly what
arranging streams
are meant for.The game developer can indicate which stream he can order his packets and how he wants to arrange them.
For example, the game developer can say: "Let me set all chat messages to 'stream 1' and all motion packets to 'stream 2'.
Usages
Imagine we have the following packet:
When this packet arrives we can check whether it is sequenced or ordered.
Next, we define two systems somewhere we can store them for a longer time.
Ordering
When an ordered packet arrives we get the stream on which the packet should be arranged. And then we arrange the packet on that stream.
The stream could either return
Some
orNone
whether the current packet is in order and previous packets are already received.Please check out ordering module for more information about how ordering works. We could also use
iter_mut()
for reading up-following packets in order.Sequencing
When a sequenced packet arrives we get the stream on which the packet should be arranged. And then we arrange the packet on that stream.
The stream could either return
Some
orNone
whether the received packet is the newest we have seen so far.Please check out sequencing module for more information about how sequencing works.
Design Decision
This section is about the design decisions I made. First, we will cover a diagram next I will walk through different ideas I had through this process.
Diagram
I figured to create a diagram, because why not. This is modeled in BPMN-modeling language. It might not be 100% correct since I am still learning to master it.
Traits
I created two traits for shared functionality.
Arranging trait
Both 'ordering' and 'sequencing' are about arranging elements. That is why I thought that this name was the best one here.
Before this name, I had to sort or ordering. But ordering was not suitable because the term order was already in use by ordering packages themselves.
I also decided not to use sorting because I had to think of sorting a range of items; but in our system, it's not about sorting a range but about arranging incoming packets.
This trait has a function 'arrange' which is called when a packet is received.
This trait has two implementations:
ArrangingSystem trait
An arranging system is about managing the different 'arranging' streams as we spook of before.
So how do we name a type that manages the individual streams? To me
ArraningSystem
was convenient enough.This trait has two implementations:
Data Structures
I had to make choices about which data structures I would use for storing data. Below is a summary of the research I did.
ArrangingStreams Storage
Both in
OrderingSystem
and inSequencingSystem
we'll have to store streams.I used
HashMap
here, because, a stream is identified by an id, and no more than 255 streams will be created, and most operations performed with them are reads.Because reading is one of a
Hashmaps
I found no other reason to use it as goto-storage.Packets Storage
We needed a place to store packages for ordering. Each packet that is ordered can be identified with an
incomming_index
(sequence number), this id is stored together with a value.When we are talking about sorting packets we could take the amount of 1024 as a reference point.
Choosing a data structure.
There were a number of possibilities for storing packets.
I have looked at several (rust) libraries read some documentation and played around my self with some of those concepts, to form a standpoint of why I would choose one over another.
At first, I state that I prefer to stay as far as possible from external libraries unless there is no other option.
1. CircularBuffer
Pros:
Cons:
2. CicularQueueue
Pros:
Cons:
I have created two implementations for this
CircularQueue
:Hashmap
Pros:
Cons:
Librarys
I checkedout the following librarys for insperation:
Those libraries were dealing with the same cons and pro's from above-listed points and some of them use unsafe code to accomplish things. Some offer more and some will bring more dependencies into laminar than we need.
Conclusion
Some allocating issues could be solved by manually playing with unsafe code. However, since I am not that confident with allocating, and I could probably say this for everybody here, we could better avoid it as much as possible.
The change for errors in our program will be much bigger with this.
I think the hashmap here is the best choice it is easy to implement, easy to use and has some important gains over other data structures.
Sources I spend some time with:
https://en.wikipedia.org/wiki/Priority_queue
https://github.com/erizocosmico/ring_queue/blob/master/src/lib.rs
https://www.studytonight.com/data-structures/circular-queue
https://www.geeksforgeeks.org/circular-queue-set-1-introduction-array-implementation/
https://www.geeksforgeeks.org/priority-queue-set-1-introduction/
Tests
I wrote a bunch of unit tests to assure the correctness of the code.
With those unit tests I introduced two macros:
First one asserts that the given collection, on the left, should result - after it is sequenced - into the given collection, on the right.
First one asserts that the given collection, on the left, should result - after it is ordered - into the given collection, on the right.
The above two macros allow us to control the functionality in a more pleasant way.
Notices