Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2047273002:
tracing v2: Add ScatteredStreamWriter (Closed)
|
Created:
4 years, 6 months ago by Primiano Tucci (use gerrit) Modified:
4 years, 5 months ago CC:
chromium-reviews, tracing+reviews_chromium.org, wfh+watch_chromium.org Base URL:
https://chromium.googlesource.com/chromium/src.git@tv_ups1 Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
Descriptiontracing v2: Add ScatteredStreamWriter
ScatteredStreamWriter is the core class that will be used to
serialize proto messages in flight into the trace buffer chunks.
This class handles writes and spreads them over non-contiguous
memory chunks without causing extra copies.
BUG=608719
Committed: https://crrev.com/ef9be0368adbf4cff57b908f2bb5ed67db4a18c3
Cr-Commit-Position: refs/heads/master@{#403177}
Patch Set 1 #
Total comments: 13
Patch Set 2 : . #Patch Set 3 : . #Patch Set 4 : . #
Total comments: 36
Patch Set 5 : Meh, <algorithm> required for std::min, ooh la la #Patch Set 6 : petrcermak review #
Total comments: 2
Patch Set 7 : Typo #
Depends on Patchset: Messages
Total messages: 22 (11 generated)
Description was changed from ========== tracing v2: Add ScatteredStreamWriter ScatteredStreamWriter is the core class that will be used by proto messages to serialize on the flight in the trace buffer chunks. The major deal of this class is handling writes spreading over non-contiguous memory chunks. BUG=608719 ========== to ========== tracing v2: Add ScatteredStreamWriter ScatteredStreamWriter is the core class that will be used by proto messages to serialize on the flight in the trace buffer chunks. The major deal of this class is handling writes and spreading them over non-contiguous memory chunks. BUG=608719 ==========
primiano@chromium.org changed reviewers: + kraynov@chromium.org, oysteine@chromium.org
2nd piece after https://codereview.chromium.org/2049653002/
I think the design is great, especially being agnostic to the underlying buffer. Minor comments only! https://codereview.chromium.org/2047273002/diff/1/components/tracing/core/sca... File components/tracing/core/scattered_stream_writer.cc (right): https://codereview.chromium.org/2047273002/diff/1/components/tracing/core/sca... components/tracing/core/scattered_stream_writer.cc:28: void ScatteredStreamWriter::Expand() { Maybe Extend() instead? Or Advance() to signal that it's not actually getting bigger. https://codereview.chromium.org/2047273002/diff/1/components/tracing/core/sca... components/tracing/core/scattered_stream_writer.cc:43: // a tracing hot path. Looks like this is faster than memcpy for small Huh interesting (and surprising). Any ideas what the threshold is? It'd be interesting to see overloads for constant sizes, if we'll mainly be dealing with a few specific ones, so the loop could be unrolled. https://codereview.chromium.org/2047273002/diff/1/components/tracing/core/sca... File components/tracing/core/scattered_stream_writer.h (right): https://codereview.chromium.org/2047273002/diff/1/components/tracing/core/sca... components/tracing/core/scattered_stream_writer.h:8: #include <inttypes.h> Can we rely on this? Unsure what the status of C99 is. https://codereview.chromium.org/2047273002/diff/1/components/tracing/core/sca... components/tracing/core/scattered_stream_writer.h:22: // occasions these writes nees to be spread over two (or more) non contiguous nit: need https://codereview.chromium.org/2047273002/diff/1/components/tracing/core/sca... components/tracing/core/scattered_stream_writer.h:55: // reuqesting a new contiguous buffer. nit: requesting
alph@chromium.org changed reviewers: + alph@chromium.org
https://codereview.chromium.org/2047273002/diff/1/components/tracing/core/sca... File components/tracing/core/scattered_stream_writer.cc (right): https://codereview.chromium.org/2047273002/diff/1/components/tracing/core/sca... components/tracing/core/scattered_stream_writer.cc:43: // a tracing hot path. Looks like this is faster than memcpy for small On 2016/06/15 08:35:09, Oystein wrote: > Huh interesting (and surprising). Any ideas what the threshold is? It'd be > interesting to see overloads for constant sizes, if we'll mainly be dealing with > a few specific ones, so the loop could be unrolled. Compiler should be able to handle this optimization. Back from my compiler dev days, I remember there were dozens of memcpy expansions paths for various sizes and CPUs. Having size to be a template argument might help if there are cases when the size is known at compile time. https://codereview.chromium.org/2047273002/diff/1/components/tracing/core/sca... components/tracing/core/scattered_stream_writer.cc:72: write_ptr_ += size; Some bytes may be left at the end of the previous chunk. are we fine with that?
primiano@chromium.org changed reviewers: + petrcermak@chromium.org
+petrcermak https://codereview.chromium.org/2047273002/diff/1/components/tracing/core/sca... File components/tracing/core/scattered_stream_writer.cc (right): https://codereview.chromium.org/2047273002/diff/1/components/tracing/core/sca... components/tracing/core/scattered_stream_writer.cc:28: void ScatteredStreamWriter::Expand() { On 2016/06/15 08:35:09, Oystein (OOO to July11) wrote: > Maybe Extend() instead? Or Advance() to signal that it's not actually getting > bigger. Right. Expand is the worst word given that nothing is shrinking/expanding. https://codereview.chromium.org/2047273002/diff/1/components/tracing/core/sca... components/tracing/core/scattered_stream_writer.cc:43: // a tracing hot path. Looks like this is faster than memcpy for small On 2016/06/15 10:08:22, alph wrote: > On 2016/06/15 08:35:09, Oystein wrote: > > Huh interesting (and surprising). Any ideas what the threshold is? It'd be > > interesting to see overloads for constant sizes, if we'll mainly be dealing > with > > a few specific ones, so the loop could be unrolled. > > Compiler should be able to handle this optimization. > Back from my compiler dev days, I remember there were dozens of memcpy > expansions paths for various sizes and CPUs. > > Having size to be a template argument might help if there are cases when the > size is known at compile time. Ok let's do something, I want to look with a fresh mind to this. Let's use memcpy for the moment and leave the todo for the perf optimization. I agree with alph, I think that the template will help here. I think we just need the template for size 2 and 4 that will be the more common, and for the rest we can use this version. EDIT: looks like templating the size is the key. If I pass a constexpr arg to memcpy it becomes 2x faster on Mac (even w.r.t the home brewed loop). I think what's happening is that the generic version of memcpy has too many branches to figure out the smart path which cost outweight a simple loop. https://codereview.chromium.org/2047273002/diff/1/components/tracing/core/sca... components/tracing/core/scattered_stream_writer.cc:72: write_ptr_ += size; On 2016/06/15 10:08:22, alph wrote: > Some bytes may be left at the end of the previous chunk. are we fine with that? Yes, so, are you talking about the fact that the last bytes of a chunk might be left uninitialized, right? It is intentional, reason why the chunk has the set_used_size() which writes the payload size in the header. We can ever guarantee that a chunk is fully packed. For instance we might be in the state where there are 2 bytes left and we want to add a new event. But a new event requires at least the 1 byte proto preamble (to tell the type) plus the size, plus 2 bytes to tell that the event is not complete. https://codereview.chromium.org/2047273002/diff/1/components/tracing/core/sca... File components/tracing/core/scattered_stream_writer.h (right): https://codereview.chromium.org/2047273002/diff/1/components/tracing/core/sca... components/tracing/core/scattered_stream_writer.h:8: #include <inttypes.h> On 2016/06/15 08:35:09, Oystein (OOO to July11) wrote: > Can we rely on this? Unsure what the status of C99 is. ehm, here I really meant stdint (for uint32_t etc), brain collision. (Btw lot of the codebase seems to depend on inttypes so I think we de-facto can :)). Anyways, removing in favor of stindt, that's what I really meant here. https://codereview.chromium.org/2047273002/diff/1/components/tracing/core/sca... components/tracing/core/scattered_stream_writer.h:22: // occasions these writes nees to be spread over two (or more) non contiguous On 2016/06/15 08:35:10, Oystein (OOO to July11) wrote: > nit: need Done. https://codereview.chromium.org/2047273002/diff/1/components/tracing/core/sca... components/tracing/core/scattered_stream_writer.h:55: // reuqesting a new contiguous buffer. On 2016/06/15 08:35:09, Oystein (OOO to July11) wrote: > nit: requesting Done.
Patch description comments: 1) I don't understand what "will be used BY proto messages to serialize ON the flight IN the trace buffer chunks" means. Do you mean "will be used to serialize proto messages IN flight INTO trace buffer chunks"? 2) "The major deal WITH this class ..." Thanks, Petr https://codereview.chromium.org/2047273002/diff/60001/components/tracing/core... File components/tracing/core/scattered_stream_writer.cc (right): https://codereview.chromium.org/2047273002/diff/60001/components/tracing/core... components/tracing/core/scattered_stream_writer.cc:32: if (write_ptr_ >= cur_range_.end) You do this in 2 places. Assuming the compiler inlines it (performance), I think it make sense to have an ExtendIfNecessary() method instead. https://codereview.chromium.org/2047273002/diff/60001/components/tracing/core... components/tracing/core/scattered_stream_writer.cc:39: if (end <= cur_range_.end) { You could make this method simpler and avoid recursion by simply using the loop: while (size > 0) { if (write_ptr_ >= cur_range_.end) Extend(); const size_t burst_size = std::min(bytes_available(), size); memcpy(write_ptr, src, burst_size); size -= burst_size; src += burst_size; } https://codereview.chromium.org/2047273002/diff/60001/components/tracing/core... components/tracing/core/scattered_stream_writer.cc:46: return; you use both return and else. only one is necessary https://codereview.chromium.org/2047273002/diff/60001/components/tracing/core... components/tracing/core/scattered_stream_writer.cc:71: return {begin, begin + size}; You need to check that |size| is not greater than the size of the chunk. Otherwise, you've just returned an invalid range of memory!!! https://codereview.chromium.org/2047273002/diff/60001/components/tracing/core... File components/tracing/core/scattered_stream_writer.h (right): https://codereview.chromium.org/2047273002/diff/60001/components/tracing/core... components/tracing/core/scattered_stream_writer.h:25: // advance what the size of the message will be. nit: The subject of this sentence is "proto messages", so this should probably be "what THEIR size will be". https://codereview.chromium.org/2047273002/diff/60001/components/tracing/core... components/tracing/core/scattered_stream_writer.h:26: // Due to the tracing buffer being split into fixed-size chunks, in some nit: I think that the correct wording is "ON ... occasions" https://codereview.chromium.org/2047273002/diff/60001/components/tracing/core... components/tracing/core/scattered_stream_writer.h:27: // occasions these writes need to be spread over two (or more) non contiguous nit: s/non contiguous/non-contiguous/ https://codereview.chromium.org/2047273002/diff/60001/components/tracing/core... components/tracing/core/scattered_stream_writer.h:27: // occasions these writes need to be spread over two (or more) non contiguous nit: I think there should be a comma after "occasions". https://codereview.chromium.org/2047273002/diff/60001/components/tracing/core... components/tracing/core/scattered_stream_writer.h:29: // to avoid realloc() calls, as they can cause an expensive memcpy to extend. FYI, I have no idea what it means when "an expensive memcpy extends". Maybe be a little more explanatory? https://codereview.chromium.org/2047273002/diff/60001/components/tracing/core... components/tracing/core/scattered_stream_writer.h:32: // ContiguousMemoryRange, and defers the chunk-chaining logic to the Delegate. nit: there shouldn't be a comma here https://codereview.chromium.org/2047273002/diff/60001/components/tracing/core... components/tracing/core/scattered_stream_writer.h:46: // Reserves a fixed number of bytes, to be backfilled later. nit: the comma here is confusing since it incorrectly implies that "to be backfilled later" does not rely to "bytes" https://codereview.chromium.org/2047273002/diff/60001/components/tracing/core... components/tracing/core/scattered_stream_writer.h:47: ContiguousMemoryRange ReserveBytes(size_t size); Even though it's technically already in the type name, I think that the method should be called "ReserveContiguousBytes" (and probably also mention it in the comment). https://codereview.chromium.org/2047273002/diff/60001/components/tracing/core... File components/tracing/core/scattered_stream_writer_unittest.cc (right): https://codereview.chromium.org/2047273002/diff/60001/components/tracing/core... components/tracing/core/scattered_stream_writer_unittest.cc:51: ssw.WriteByte(i); you could also add EXPECT_EQ(kChunkSize - i - 1, ssw.bytes_available()); https://codereview.chromium.org/2047273002/diff/60001/components/tracing/core... components/tracing/core/scattered_stream_writer_unittest.cc:60: // This starts at offset 1 , to make sure we don't hardcode any assumption nit: remove space and comma https://codereview.chromium.org/2047273002/diff/60001/components/tracing/core... components/tracing/core/scattered_stream_writer_unittest.cc:63: EXPECT_EQ(3u, ssw.bytes_available()); check chunk count as well https://codereview.chromium.org/2047273002/diff/60001/components/tracing/core... components/tracing/core/scattered_stream_writer_unittest.cc:67: EXPECT_EQ(3u, delegate.chunks.size()); check bytes_available as well https://codereview.chromium.org/2047273002/diff/60001/components/tracing/core... components/tracing/core/scattered_stream_writer_unittest.cc:77: EXPECT_EQ(7u, ssw.bytes_available()); check chunk count as well https://codereview.chromium.org/2047273002/diff/60001/components/tracing/core... components/tracing/core/scattered_stream_writer_unittest.cc:84: EXPECT_EQ(7u, delegate.chunks.size()); check bytes_available as well
Description was changed from ========== tracing v2: Add ScatteredStreamWriter ScatteredStreamWriter is the core class that will be used by proto messages to serialize on the flight in the trace buffer chunks. The major deal of this class is handling writes and spreading them over non-contiguous memory chunks. BUG=608719 ========== to ========== tracing v2: Add ScatteredStreamWriter ScatteredStreamWriter is the core class that will be used to serialize proto messages in flight into the trace buffer chunks. This class handles writes and spreads them over non-contiguous memory chunks without causing extra copies. BUG=608719 ==========
Description was changed from ========== tracing v2: Add ScatteredStreamWriter ScatteredStreamWriter is the core class that will be used to serialize proto messages in flight into the trace buffer chunks. This class handles writes and spreads them over non-contiguous memory chunks without causing extra copies. BUG=608719 ========== to ========== tracing v2: Add ScatteredStreamWriter ScatteredStreamWriter is the core class that will be used to serialize proto messages in flight into the trace buffer chunks. This class handles writes and spreads them over non-contiguous memory chunks without causing extra copies. BUG=608719 ==========
Thanks. Replies inline + new patchset. https://codereview.chromium.org/2047273002/diff/60001/components/tracing/core... File components/tracing/core/scattered_stream_writer.cc (right): https://codereview.chromium.org/2047273002/diff/60001/components/tracing/core... components/tracing/core/scattered_stream_writer.cc:32: if (write_ptr_ >= cur_range_.end) On 2016/06/29 10:27:48, petrcermak wrote: > You do this in 2 places. Assuming the compiler inlines it (performance), I think > it make sense to have an ExtendIfNecessary() method instead. I thought to that. Is not about perf and inlining. I just think that this is more readable having the explicit if. otherwise in order to figure out what ExpandIfNecessary does your eyes have to hop on 3 places. (ExpandIfNecessary -> Expand() -> Reset(NewBuffer)). Since there are only 2 cases, code duplication is not that much w.r.t. the resulting unreadability. More practically known as "Rule of Three": https://en.wikipedia.org/wiki/Rule_of_three_(computer_programming) https://codereview.chromium.org/2047273002/diff/60001/components/tracing/core... components/tracing/core/scattered_stream_writer.cc:39: if (end <= cur_range_.end) { On 2016/06/29 10:27:48, petrcermak wrote: > You could make this method simpler and avoid recursion by simply using the loop: > > while (size > 0) { > if (write_ptr_ >= cur_range_.end) > Extend(); > const size_t burst_size = std::min(bytes_available(), size); > memcpy(write_ptr, src, burst_size); > size -= burst_size; > src += burst_size; > } I know, but that would have 3 if-s in the fastpath (one hidden in the std::min), mine has one. It's just about perf and helping the branch predictor. This path will really be hit a lot, perf shows this the hotspot of tracing (unsurprisingly). slighly reformatted this, the else block wasn't really required after the else. https://codereview.chromium.org/2047273002/diff/60001/components/tracing/core... components/tracing/core/scattered_stream_writer.cc:46: return; On 2016/06/29 10:27:48, petrcermak wrote: > you use both return and else. only one is necessary right. done. https://codereview.chromium.org/2047273002/diff/60001/components/tracing/core... components/tracing/core/scattered_stream_writer.cc:71: return {begin, begin + size}; On 2016/06/29 10:27:48, petrcermak wrote: > You need to check that |size| is not greater than the size of the chunk. > Otherwise, you've just returned an invalid range of memory!!! Correct, that was just an assumption, perhaps too implicit and undocumented. Added a DCHECK, but will remove later it once this becomes a fixed-size function with no size argument, this will be a dcheck in a fast path that will make debug uber slow.. https://codereview.chromium.org/2047273002/diff/60001/components/tracing/core... File components/tracing/core/scattered_stream_writer.h (right): https://codereview.chromium.org/2047273002/diff/60001/components/tracing/core... components/tracing/core/scattered_stream_writer.h:25: // advance what the size of the message will be. On 2016/06/29 10:27:48, petrcermak wrote: > nit: The subject of this sentence is "proto messages", so this should probably > be "what THEIR size will be". Done. https://codereview.chromium.org/2047273002/diff/60001/components/tracing/core... components/tracing/core/scattered_stream_writer.h:26: // Due to the tracing buffer being split into fixed-size chunks, in some On 2016/06/29 10:27:48, petrcermak wrote: > nit: I think that the correct wording is "ON ... occasions" but.. there are 6 ""in some occasions" vs 1 "on some occasions" in codesearch :) (a search on google though shows a majority of "on"). Ok ok https://codereview.chromium.org/2047273002/diff/60001/components/tracing/core... components/tracing/core/scattered_stream_writer.h:27: // occasions these writes need to be spread over two (or more) non contiguous On 2016/06/29 10:27:48, petrcermak wrote: > nit: s/non contiguous/non-contiguous/ Done. https://codereview.chromium.org/2047273002/diff/60001/components/tracing/core... components/tracing/core/scattered_stream_writer.h:27: // occasions these writes need to be spread over two (or more) non contiguous On 2016/06/29 10:27:48, petrcermak wrote: > nit: s/non contiguous/non-contiguous/ Done. https://codereview.chromium.org/2047273002/diff/60001/components/tracing/core... components/tracing/core/scattered_stream_writer.h:29: // to avoid realloc() calls, as they can cause an expensive memcpy to extend. On 2016/06/29 10:27:48, petrcermak wrote: > FYI, I have no idea what it means when "an expensive memcpy extends". Maybe be a > little more explanatory? Done. https://codereview.chromium.org/2047273002/diff/60001/components/tracing/core... components/tracing/core/scattered_stream_writer.h:32: // ContiguousMemoryRange, and defers the chunk-chaining logic to the Delegate. On 2016/06/29 10:27:48, petrcermak wrote: > nit: there shouldn't be a comma here Done. https://codereview.chromium.org/2047273002/diff/60001/components/tracing/core... components/tracing/core/scattered_stream_writer.h:46: // Reserves a fixed number of bytes, to be backfilled later. On 2016/06/29 10:27:48, petrcermak wrote: > nit: the comma here is confusing since it incorrectly implies that "to be > backfilled later" does not rely to "bytes" Ups. Yeah I think it's a refuse from an older longer comment. https://codereview.chromium.org/2047273002/diff/60001/components/tracing/core... components/tracing/core/scattered_stream_writer.h:47: ContiguousMemoryRange ReserveBytes(size_t size); On 2016/06/29 10:27:49, petrcermak wrote: > Even though it's technically already in the type name, I think that the method > should be called "ReserveContiguousBytes" (and probably also mention it in the > comment). It's funny because the previous name was ReserveContiguous bytes but then I thought that it was redundant as the return value makes it clear. Adding just a comment. https://codereview.chromium.org/2047273002/diff/60001/components/tracing/core... File components/tracing/core/scattered_stream_writer_unittest.cc (right): https://codereview.chromium.org/2047273002/diff/60001/components/tracing/core... components/tracing/core/scattered_stream_writer_unittest.cc:51: ssw.WriteByte(i); On 2016/06/29 10:27:49, petrcermak wrote: > you could also add EXPECT_EQ(kChunkSize - i - 1, ssw.bytes_available()); Done. https://codereview.chromium.org/2047273002/diff/60001/components/tracing/core... components/tracing/core/scattered_stream_writer_unittest.cc:60: // This starts at offset 1 , to make sure we don't hardcode any assumption On 2016/06/29 10:27:49, petrcermak wrote: > nit: remove space and comma Done. https://codereview.chromium.org/2047273002/diff/60001/components/tracing/core... components/tracing/core/scattered_stream_writer_unittest.cc:63: EXPECT_EQ(3u, ssw.bytes_available()); On 2016/06/29 10:27:49, petrcermak wrote: > check chunk count as well Done. https://codereview.chromium.org/2047273002/diff/60001/components/tracing/core... components/tracing/core/scattered_stream_writer_unittest.cc:67: EXPECT_EQ(3u, delegate.chunks.size()); On 2016/06/29 10:27:49, petrcermak wrote: > check bytes_available as well Done. https://codereview.chromium.org/2047273002/diff/60001/components/tracing/core... components/tracing/core/scattered_stream_writer_unittest.cc:77: EXPECT_EQ(7u, ssw.bytes_available()); On 2016/06/29 10:27:49, petrcermak wrote: > check chunk count as well Done. https://codereview.chromium.org/2047273002/diff/60001/components/tracing/core... components/tracing/core/scattered_stream_writer_unittest.cc:84: EXPECT_EQ(7u, delegate.chunks.size()); On 2016/06/29 10:27:49, petrcermak wrote: > check bytes_available as well Done.
Patchset #6 (id:100001) has been deleted
LGTM. Thanks, Petr https://codereview.chromium.org/2047273002/diff/120001/components/tracing/cor... File components/tracing/core/scattered_stream_writer.cc (right): https://codereview.chromium.org/2047273002/diff/120001/components/tracing/cor... components/tracing/core/scattered_stream_writer.cc:66: nit: this blank line seems unnecessary (it actually makes the code less readable in my opinion).
https://codereview.chromium.org/2047273002/diff/120001/components/tracing/cor... File components/tracing/core/scattered_stream_writer.cc (right): https://codereview.chromium.org/2047273002/diff/120001/components/tracing/cor... components/tracing/core/scattered_stream_writer.cc:66: On 2016/06/30 12:39:04, petrcermak wrote: > nit: this blank line seems unnecessary (it actually makes the code less readable > in my opinion). Oops agree. in fact that is just a typo accident. Done.
The CQ bit was checked by primiano@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from petrcermak@chromium.org Link to the patchset: https://codereview.chromium.org/2047273002/#ps140001 (title: "Typo")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Description was changed from ========== tracing v2: Add ScatteredStreamWriter ScatteredStreamWriter is the core class that will be used to serialize proto messages in flight into the trace buffer chunks. This class handles writes and spreads them over non-contiguous memory chunks without causing extra copies. BUG=608719 ========== to ========== tracing v2: Add ScatteredStreamWriter ScatteredStreamWriter is the core class that will be used to serialize proto messages in flight into the trace buffer chunks. This class handles writes and spreads them over non-contiguous memory chunks without causing extra copies. BUG=608719 ==========
Message was sent while issue was closed.
Committed patchset #7 (id:140001)
Message was sent while issue was closed.
Description was changed from ========== tracing v2: Add ScatteredStreamWriter ScatteredStreamWriter is the core class that will be used to serialize proto messages in flight into the trace buffer chunks. This class handles writes and spreads them over non-contiguous memory chunks without causing extra copies. BUG=608719 ========== to ========== tracing v2: Add ScatteredStreamWriter ScatteredStreamWriter is the core class that will be used to serialize proto messages in flight into the trace buffer chunks. This class handles writes and spreads them over non-contiguous memory chunks without causing extra copies. BUG=608719 Committed: https://crrev.com/ef9be0368adbf4cff57b908f2bb5ed67db4a18c3 Cr-Commit-Position: refs/heads/master@{#403177} ==========
Message was sent while issue was closed.
Patchset 7 (id:??) landed as https://crrev.com/ef9be0368adbf4cff57b908f2bb5ed67db4a18c3 Cr-Commit-Position: refs/heads/master@{#403177} |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
