Refactor rtt [3/2] #573
Refactor rtt [3/2] #573
Conversation
Urhengulas
force-pushed
the
refactor-rtt
branch
from
39c3bee
to
b130555
Compare
574: Refactor rtt [1/2] r=jonas-schievink a=Urhengulas First half of #573: * defmt-rtt: Move struct Channel into own module * defmt-rtt: Make fn host_is_connected a method of Channel * defmt-rtt: Move struct Header into own module * defmt-rtt: Rename fn handle to fn channel + mv to mod channel Co-authored-by: Johann Hemmann <[email protected]>
Urhengulas
force-pushed
the
refactor-rtt
branch
from
b130555
to
03a5b05
Compare
Urhengulas
requested review from
japaric
and removed request for
jonas-schievink
| /// | ||
| /// Returns the amount of bytes copied. | ||
| fn write_impl(&self, bytes: &[u8], available: usize) -> usize { | ||
| let cursor = self.write.load(Ordering::Acquire); |
There was a problem hiding this comment.
if you do an atomic load here you'll end up doing two atomic loads of the write cursor in the blocking_write method. the compiler never merges atomic operations so this will end up adding more instructions and make the blocking_write operations slower.
There was a problem hiding this comment.
So we rather add it as another argument to the function?
There was a problem hiding this comment.
maybe. the main point here is that if you reorder atomic operations, add new ones or move memory operations around the atomic operations you are no longer refactoring the code: you are subtly changing its semantics.
There was a problem hiding this comment.
I get that. I've split out the not-error-prone changes to #622 and converted this one to a draft.
I'd like to keep it open, because it is a quite obvious case of code-duplication.
622: Refactor rtt [2/2] r=Urhengulas a=Urhengulas This PR contains the not-error-prone changes from #573 and addresses the review given to #573. 661: `snapshot-tests`: Add tests for cell types r=Urhengulas a=Urhengulas Follow-up to #656, which add snapshot tests for the new `defmt::Format` implementations. Co-authored-by: Johann Hemmann <[email protected]>
695: `defmt-rtt`: Refactor rtt [3/2] r=Urhengulas a=Urhengulas This is the second attempt to land #573. This time I am not touching atomic variables, but only de-duplicate the almost copy pasted parts of `blocking_write` and `nonblocking_write`. Especially the first commit makes the duplication quite clear. Co-authored-by: Urhengulas <[email protected]>
This PR mainly factors out the common part of
fn blocking_writeandfn nonblocking_writeinto one common method.