Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1256053006:
Protect AudioScheduledSoureNode::m_startTime and m_endTime. (Closed)
|
Created:
5 years, 4 months ago by Raymond Toy Modified:
4 years, 7 months ago CC:
blink-reviews, blink-reviews-wtf_chromium.org, Mikhail Base URL:
https://chromium.googlesource.com/chromium/blink.git@master Target Ref:
refs/heads/master Project:
blink Visibility:
Public. |
DescriptionProtect AudioScheduledSoureNode::m_startTime and m_endTime.
Both AudioScheduledSourceNode::m_startTime and m_endTime can be
accessed by the main thread and the audio thread. The main thread
sets these when the node is scheduled to start or stop. The audio
thread needs to read these to determine if the node should actually
start or stop.
Update AudioScheduledSource::start/stop and AudioBufferSource::start to
set a lock when modifying m_startTime or m_endTime.
BUG=516261
Committed: https://crrev.com/3165ed4939f10c1768fbfcf8cf894da3b5295d8e
git-svn-id: svn://svn.chromium.org/blink/trunk@202152 bbb929c8-8fbe-4397-9dbb-9b2b20218538
Patch Set 1 #Patch Set 2 : Rebase #Patch Set 3 : Rebase #
Total comments: 5
Patch Set 4 : Add static_assert #
Total comments: 4
Patch Set 5 : Use tryLock instead of atomic access #
Total comments: 1
Patch Set 6 : Revert change to OscillatorNode.cpp #
Messages
Total messages: 31 (4 generated)
rtoy@chromium.org changed reviewers: + hongchan@chromium.org
rtoy@chromium.org changed reviewers: + glider@chromium.org, tkent@chromium.org
PTAL. hoch@: webaudio tkent@, glider@: Atomics.h support for float and double.
webaudio part LGTM.
https://codereview.chromium.org/1256053006/diff/40001/Source/modules/webaudio... File Source/modules/webaudio/AudioScheduledSourceNode.h (right): https://codereview.chromium.org/1256053006/diff/40001/Source/modules/webaudio... Source/modules/webaudio/AudioScheduledSourceNode.h:111: return acquireLoad(&m_startTime); I'm not sure acquireLoad and releaseStore are enough for 64 bit value on 32 bit platforms. https://codereview.chromium.org/1256053006/diff/40001/Source/wtf/Atomics.h File Source/wtf/Atomics.h (right): https://codereview.chromium.org/1256053006/diff/40001/Source/wtf/Atomics.h#ne... Source/wtf/Atomics.h:167: union { should have static_assert for sizeof(int) == sizeof(float).
https://codereview.chromium.org/1256053006/diff/40001/Source/modules/webaudio... File Source/modules/webaudio/AudioScheduledSourceNode.h (right): https://codereview.chromium.org/1256053006/diff/40001/Source/modules/webaudio... Source/modules/webaudio/AudioScheduledSourceNode.h:111: return acquireLoad(&m_startTime); On 2015/08/07 01:10:03, tkent wrote: > I'm not sure acquireLoad and releaseStore are enough for 64 bit value on 32 bit > platforms. Hmm. I perhaps erroneously assumed floating-point reads/writes would be atomic 64-bit operations. Do you have an alternative? If this is not enough, I guess a more coarse-grained approach will be needed. Atomics.h has support for long long, which should be 64 bits. If that works, floats and doubles should also work, right?
https://codereview.chromium.org/1256053006/diff/40001/Source/wtf/Atomics.h File Source/wtf/Atomics.h (right): https://codereview.chromium.org/1256053006/diff/40001/Source/wtf/Atomics.h#ne... Source/wtf/Atomics.h:167: union { On 2015/08/07 01:10:04, tkent wrote: > should have static_assert for sizeof(int) == sizeof(float). Done.
https://codereview.chromium.org/1256053006/diff/60001/Source/wtf/Atomics.h File Source/wtf/Atomics.h (right): https://codereview.chromium.org/1256053006/diff/60001/Source/wtf/Atomics.h#ne... Source/wtf/Atomics.h:324: unsigned long long value = *ptr; Just noticed that this function (and the corresponding releaseStore() implementation) is broken on 32-bit platforms. The compiler will most likely emit several instructions for this line (3 instructions for x86), making the access nonatomic. This function must be enabled for 64-architectures only. https://codereview.chromium.org/1256053006/diff/60001/Source/wtf/Atomics.h#ne... Source/wtf/Atomics.h:340: ALWAYS_INLINE double acquireLoad(volatile const double* ptr) I'm no expert in ARM CPUs, do you have an idea whether 32-bit ARM supports atomic 64-bit float loads/stores? From what I could find I see that LDRD is executed as a sequence of two 32-bit loads. If so, this implementation is not correct for ARM-32.
https://codereview.chromium.org/1256053006/diff/60001/Source/wtf/Atomics.h File Source/wtf/Atomics.h (right): https://codereview.chromium.org/1256053006/diff/60001/Source/wtf/Atomics.h#ne... Source/wtf/Atomics.h:324: unsigned long long value = *ptr; On 2015/08/10 14:01:30, Alexander Potapenko wrote: > Just noticed that this function (and the corresponding releaseStore() > implementation) is broken on 32-bit platforms. The compiler will most likely > emit several instructions for this line (3 instructions for x86), making the > access nonatomic. > This function must be enabled for 64-architectures only. I think a bug should be filed for that and fixed in a separate CL. Since it's defined, presumably it's being used somewhere by someone.
https://codereview.chromium.org/1256053006/diff/60001/Source/wtf/Atomics.h File Source/wtf/Atomics.h (right): https://codereview.chromium.org/1256053006/diff/60001/Source/wtf/Atomics.h#ne... Source/wtf/Atomics.h:324: unsigned long long value = *ptr; On 2015/08/10 16:36:42, Raymond Toy wrote: > On 2015/08/10 14:01:30, Alexander Potapenko wrote: > > Just noticed that this function (and the corresponding releaseStore() > > implementation) is broken on 32-bit platforms. The compiler will most likely > > emit several instructions for this line (3 instructions for x86), making the > > access nonatomic. > > This function must be enabled for 64-architectures only. > > I think a bug should be filed for that and fixed in a separate CL. Since it's > defined, presumably it's being used somewhere by someone. I filed crbug.com/519096 .
https://codereview.chromium.org/1256053006/diff/40001/Source/modules/webaudio... File Source/modules/webaudio/AudioScheduledSourceNode.h (right): https://codereview.chromium.org/1256053006/diff/40001/Source/modules/webaudio... Source/modules/webaudio/AudioScheduledSourceNode.h:111: return acquireLoad(&m_startTime); On 2015/08/07 15:45:00, Raymond Toy wrote: > On 2015/08/07 01:10:03, tkent wrote: > > I'm not sure acquireLoad and releaseStore are enough for 64 bit value on 32 > bit > > platforms. > > Hmm. I perhaps erroneously assumed floating-point reads/writes would be atomic > 64-bit operations. Do you have an alternative? If this is not enough, I guess > a more coarse-grained approach will be needed. > > Atomics.h has support for long long, which should be 64 bits. If that works, > floats and doubles should also work, right? Alternative would be a normal mutex, or a spin lock. It's possible that they block an audio rendering thread, but the block time must be very short.
On 2015/08/11 00:45:19, tkent wrote: > https://codereview.chromium.org/1256053006/diff/40001/Source/modules/webaudio... > File Source/modules/webaudio/AudioScheduledSourceNode.h (right): > > https://codereview.chromium.org/1256053006/diff/40001/Source/modules/webaudio... > Source/modules/webaudio/AudioScheduledSourceNode.h:111: return > acquireLoad(&m_startTime); > On 2015/08/07 15:45:00, Raymond Toy wrote: > > On 2015/08/07 01:10:03, tkent wrote: > > > I'm not sure acquireLoad and releaseStore are enough for 64 bit value on 32 > > bit > > > platforms. > > > > Hmm. I perhaps erroneously assumed floating-point reads/writes would be atomic > > 64-bit operations. Do you have an alternative? If this is not enough, I > guess > > a more coarse-grained approach will be needed. > > > > Atomics.h has support for long long, which should be 64 bits. If that works, > > floats and doubles should also work, right? > > Alternative would be a normal mutex, or a spin lock. > It's possible that they block an audio rendering thread, but the block time must > be very short. Is it possible to store the diff between |m_startTime| and |m_endTime| instead of |m_endTime|? Maybe a float will be enough than?
On 2015/08/11 10:18:23, Alexander Potapenko wrote: > On 2015/08/11 00:45:19, tkent wrote: > > > https://codereview.chromium.org/1256053006/diff/40001/Source/modules/webaudio... > > File Source/modules/webaudio/AudioScheduledSourceNode.h (right): > > > > > https://codereview.chromium.org/1256053006/diff/40001/Source/modules/webaudio... > > Source/modules/webaudio/AudioScheduledSourceNode.h:111: return > > acquireLoad(&m_startTime); > > On 2015/08/07 15:45:00, Raymond Toy wrote: > > > On 2015/08/07 01:10:03, tkent wrote: > > > > I'm not sure acquireLoad and releaseStore are enough for 64 bit value on > 32 > > > bit > > > > platforms. > > > > > > Hmm. I perhaps erroneously assumed floating-point reads/writes would be > atomic > > > 64-bit operations. Do you have an alternative? If this is not enough, I > > guess > > > a more coarse-grained approach will be needed. > > > > > > Atomics.h has support for long long, which should be 64 bits. If that > works, > > > floats and doubles should also work, right? > > > > Alternative would be a normal mutex, or a spin lock. > > It's possible that they block an audio rendering thread, but the block time > must > > be very short. > > Is it possible to store the diff between |m_startTime| and |m_endTime| instead > of |m_endTime|? > Maybe a float will be enough than? Unfortunately no. Nodes don't have to have a stop time. And we try hard to use doubles for time because floats aren't accurate enough with high sample rates and moderately long audio context life times.
On 2015/08/11 00:45:19, tkent wrote: > https://codereview.chromium.org/1256053006/diff/40001/Source/modules/webaudio... > File Source/modules/webaudio/AudioScheduledSourceNode.h (right): > > https://codereview.chromium.org/1256053006/diff/40001/Source/modules/webaudio... > Source/modules/webaudio/AudioScheduledSourceNode.h:111: return > acquireLoad(&m_startTime); > On 2015/08/07 15:45:00, Raymond Toy wrote: > > On 2015/08/07 01:10:03, tkent wrote: > > > I'm not sure acquireLoad and releaseStore are enough for 64 bit value on 32 > > bit > > > platforms. > > > > Hmm. I perhaps erroneously assumed floating-point reads/writes would be atomic > > 64-bit operations. Do you have an alternative? If this is not enough, I > guess > > a more coarse-grained approach will be needed. > > > > Atomics.h has support for long long, which should be 64 bits. If that works, > > floats and doubles should also work, right? > > Alternative would be a normal mutex, or a spin lock. > It's possible that they block an audio rendering thread, but the block time must > be very short. I'd like to look around a bit more to see if this there are instructions to make this work. (I think google3 has some atomic operations that will work on armv6 for 64-bit operands.) If that doesn't work out, my current plan is to put a tryLock around updateSchedulingInfo. Things might then get started or stopped a bit later than expected, but if developers are scheduling sources that close to the requested time, they've already lost.
Looks like it's possible to make 64-bit atomic loads and stores iff ldrexd and strexd instructions are available, see http://gperftools.googlecode.com/svn/trunk/src/base/atomicops-internals-arm-v...
On 2015/08/11 17:12:27, Alexander Potapenko wrote: > Looks like it's possible to make 64-bit atomic loads and stores iff ldrexd and > strexd instructions are available, see > http://gperftools.googlecode.com/svn/trunk/src/base/atomicops-internals-arm-v... I looked at the flags used when compiling Chrome on Android: -march=armv7-a -mtune=generic-armv7-a I think it's safe to assume that ldrexd and strexd instructions are available. And http://gperftools.googlecode.com/svn/trunk/src/base/atomicops-internals-x86.h seems to have some code for 64-bit load/store on a 32-bit machine.
On 2015/08/11 19:44:20, Raymond Toy wrote: > On 2015/08/11 17:12:27, Alexander Potapenko wrote: > > Looks like it's possible to make 64-bit atomic loads and stores iff ldrexd and > > strexd instructions are available, see > > > http://gperftools.googlecode.com/svn/trunk/src/base/atomicops-internals-arm-v... > > I looked at the flags used when compiling Chrome on Android: -march=armv7-a > -mtune=generic-armv7-a > > I think it's safe to assume that ldrexd and strexd instructions are available. > > And > http://gperftools.googlecode.com/svn/trunk/src/base/atomicops-internals-x86.h > seems to have some code for 64-bit load/store on a 32-bit machine. Having those is nice, but we need to make sure the developers understand their cost. A relaxed load of a 64-bit value is free on x86_64 which is used for development, and people may think it is so on other platforms like 32-bit ARM.
On 2015/08/11 21:26:49, Alexander Potapenko wrote: > On 2015/08/11 19:44:20, Raymond Toy wrote: > > On 2015/08/11 17:12:27, Alexander Potapenko wrote: > > > Looks like it's possible to make 64-bit atomic loads and stores iff ldrexd > and > > > strexd instructions are available, see > > > > > > http://gperftools.googlecode.com/svn/trunk/src/base/atomicops-internals-arm-v... > > > > I looked at the flags used when compiling Chrome on Android: -march=armv7-a > > -mtune=generic-armv7-a > > > > I think it's safe to assume that ldrexd and strexd instructions are available. > > > > And > > http://gperftools.googlecode.com/svn/trunk/src/base/atomicops-internals-x86.h > > seems to have some code for 64-bit load/store on a 32-bit machine. > > Having those is nice, but we need to make sure the developers understand their > cost. A relaxed load of a 64-bit value is free on x86_64 which is used for > development, and people may think it is so on other platforms like 32-bit ARM. Perhaps so, but this is probably no worse than synchronizing with mutexes and what not and potentially much faster. In any case, how should I proceed here? Do we want to add something like this? If so, I think that should be a different CL. Also, can't we use the same kind of primitives in src/base/atomicops.h? C++11 has some memory order and atomic operations too, so could build from there as well. (I don't remember if we're building with C++11 everywhere yet.)
On 2015/08/11 22:07:02, Raymond Toy wrote: > On 2015/08/11 21:26:49, Alexander Potapenko wrote: > > On 2015/08/11 19:44:20, Raymond Toy wrote: > > > On 2015/08/11 17:12:27, Alexander Potapenko wrote: > > > > Looks like it's possible to make 64-bit atomic loads and stores iff ldrexd > > and > > > > strexd instructions are available, see > > > > > > > > > > http://gperftools.googlecode.com/svn/trunk/src/base/atomicops-internals-arm-v... > > > > > > I looked at the flags used when compiling Chrome on Android: -march=armv7-a > > > -mtune=generic-armv7-a > > > > > > I think it's safe to assume that ldrexd and strexd instructions are > available. > > > > > > And > > > > http://gperftools.googlecode.com/svn/trunk/src/base/atomicops-internals-x86.h > > > seems to have some code for 64-bit load/store on a 32-bit machine. > > > > Having those is nice, but we need to make sure the developers understand their > > cost. A relaxed load of a 64-bit value is free on x86_64 which is used for > > development, and people may think it is so on other platforms like 32-bit ARM. > > Perhaps so, but this is probably no worse than synchronizing with mutexes and > what not and potentially much faster. Agreed. > In any case, how should I proceed here? Do we want to add something like this? > If so, I think that should be a different CL. How about you first add 64-bit atomics in a different CL, then rebase your CL and use them to load and store floats? > Also, can't we use the same kind > of primitives in src/base/atomicops.h? Do you mean letting Chrome devs outside WK use 64-bit atomics on 32-bit systems? I'm not sure there've been requests for those. > C++11 has some memory order and atomic > operations too, so could build from there as well. (I don't remember if we're > building with C++11 everywhere yet.) There's a plan to switch Chromium to C++11 atomics (crbug.com/423074), but we aren't there yet.
On 2015/08/12 13:28:13, Alexander Potapenko wrote: > On 2015/08/11 22:07:02, Raymond Toy wrote: > > On 2015/08/11 21:26:49, Alexander Potapenko wrote: > > > On 2015/08/11 19:44:20, Raymond Toy wrote: > > > > On 2015/08/11 17:12:27, Alexander Potapenko wrote: > > > > > Looks like it's possible to make 64-bit atomic loads and stores iff > ldrexd > > > and > > > > > strexd instructions are available, see > > > > > > > > > > > > > > > http://gperftools.googlecode.com/svn/trunk/src/base/atomicops-internals-arm-v... > > > > > > > > I looked at the flags used when compiling Chrome on Android: > -march=armv7-a > > > > -mtune=generic-armv7-a > > > > > > > > I think it's safe to assume that ldrexd and strexd instructions are > > available. > > > > > > > > And > > > > > > http://gperftools.googlecode.com/svn/trunk/src/base/atomicops-internals-x86.h > > > > seems to have some code for 64-bit load/store on a 32-bit machine. > > > > > > Having those is nice, but we need to make sure the developers understand > their > > > cost. A relaxed load of a 64-bit value is free on x86_64 which is used for > > > development, and people may think it is so on other platforms like 32-bit > ARM. > > > > Perhaps so, but this is probably no worse than synchronizing with mutexes and > > what not and potentially much faster. > Agreed. > > > In any case, how should I proceed here? Do we want to add something like > this? > > If so, I think that should be a different CL. > How about you first add 64-bit atomics in a different CL, then rebase your CL > and use them to load and store floats? Yes, that's what I meant. > > > Also, can't we use the same kind > > of primitives in src/base/atomicops.h? > Do you mean letting Chrome devs outside WK use 64-bit atomics on 32-bit systems? > I'm not sure there've been requests for those. > > C++11 has some memory order and atomic > > operations too, so could build from there as well. (I don't remember if we're > > building with C++11 everywhere yet.) > There's a plan to switch Chromium to C++11 atomics (crbug.com/423074), but we > aren't there yet.
On 2015/08/12 at 15:41:22, Raymond Toy wrote: > On 2015/08/12 13:28:13, Alexander Potapenko wrote: > > On 2015/08/11 22:07:02, Raymond Toy wrote: > > > On 2015/08/11 21:26:49, Alexander Potapenko wrote: > > > > On 2015/08/11 19:44:20, Raymond Toy wrote: > > > > > On 2015/08/11 17:12:27, Alexander Potapenko wrote: > > > > > > Looks like it's possible to make 64-bit atomic loads and stores iff > > ldrexd > > > > and > > > > > > strexd instructions are available, see > > > > > > > > > > > > > > > > > > > > http://gperftools.googlecode.com/svn/trunk/src/base/atomicops-internals-arm-v... > > > > > > > > > > I looked at the flags used when compiling Chrome on Android: > > -march=armv7-a > > > > > -mtune=generic-armv7-a > > > > > > > > > > I think it's safe to assume that ldrexd and strexd instructions are > > > available. > > > > > > > > > > And > > > > > > > > http://gperftools.googlecode.com/svn/trunk/src/base/atomicops-internals-x86.h > > > > > seems to have some code for 64-bit load/store on a 32-bit machine. > > > > > > > > Having those is nice, but we need to make sure the developers understand > > their > > > > cost. A relaxed load of a 64-bit value is free on x86_64 which is used for > > > > development, and people may think it is so on other platforms like 32-bit > > ARM. > > > > > > Perhaps so, but this is probably no worse than synchronizing with mutexes and > > > what not and potentially much faster. > > Agreed. > > > > > In any case, how should I proceed here? Do we want to add something like > > this? > > > If so, I think that should be a different CL. > > How about you first add 64-bit atomics in a different CL, then rebase your CL > > and use them to load and store floats? > > Yes, that's what I meant. > > > > > Also, can't we use the same kind > > > of primitives in src/base/atomicops.h? > > Do you mean letting Chrome devs outside WK use 64-bit atomics on 32-bit systems? > > I'm not sure there've been requests for those. > > > C++11 has some memory order and atomic > > > operations too, so could build from there as well. (I don't remember if we're > > > building with C++11 everywhere yet.) > > There's a plan to switch Chromium to C++11 atomics (crbug.com/423074), but we > > aren't there yet. I think I need to abandon this approach. Chrome on Android runs on 32-bit MIPS machines and, as I understand it, there is no 64-bit atomic operation there.
On 2015/09/09 at 22:07:42, Raymond Toy wrote: > On 2015/08/12 at 15:41:22, Raymond Toy wrote: > > On 2015/08/12 13:28:13, Alexander Potapenko wrote: > > > On 2015/08/11 22:07:02, Raymond Toy wrote: > > > > On 2015/08/11 21:26:49, Alexander Potapenko wrote: > > > > > On 2015/08/11 19:44:20, Raymond Toy wrote: > > > > > > On 2015/08/11 17:12:27, Alexander Potapenko wrote: > > > > > > > Looks like it's possible to make 64-bit atomic loads and stores iff > > > ldrexd > > > > > and > > > > > > > strexd instructions are available, see > > > > > > > > > > > > > > > > > > > > > > > > > http://gperftools.googlecode.com/svn/trunk/src/base/atomicops-internals-arm-v... > > > > > > > > > > > > I looked at the flags used when compiling Chrome on Android: > > > -march=armv7-a > > > > > > -mtune=generic-armv7-a > > > > > > > > > > > > I think it's safe to assume that ldrexd and strexd instructions are > > > > available. > > > > > > > > > > > > And > > > > > > > > > > http://gperftools.googlecode.com/svn/trunk/src/base/atomicops-internals-x86.h > > > > > > seems to have some code for 64-bit load/store on a 32-bit machine. > > > > > > > > > > Having those is nice, but we need to make sure the developers understand > > > their > > > > > cost. A relaxed load of a 64-bit value is free on x86_64 which is used for > > > > > development, and people may think it is so on other platforms like 32-bit > > > ARM. > > > > > > > > Perhaps so, but this is probably no worse than synchronizing with mutexes and > > > > what not and potentially much faster. > > > Agreed. > > > > > > > In any case, how should I proceed here? Do we want to add something like > > > this? > > > > If so, I think that should be a different CL. > > > How about you first add 64-bit atomics in a different CL, then rebase your CL > > > and use them to load and store floats? > > > > Yes, that's what I meant. > > > > > > > Also, can't we use the same kind > > > > of primitives in src/base/atomicops.h? > > > Do you mean letting Chrome devs outside WK use 64-bit atomics on 32-bit systems? > > > I'm not sure there've been requests for those. > > > > C++11 has some memory order and atomic > > > > operations too, so could build from there as well. (I don't remember if we're > > > > building with C++11 everywhere yet.) > > > There's a plan to switch Chromium to C++11 atomics (crbug.com/423074), but we > > > aren't there yet. > > I think I need to abandon this approach. Chrome on Android runs on 32-bit MIPS machines and, as I understand it, there is no 64-bit atomic operation there. Some alternative approaches: 1. spinlocks for m_startTime and m_endTime 2. Box up m_startTime and m_endTime and access the pointer appropriately. When a new startTime or endTime is set, allocate a new double and update the pointer atomically. 3. Use tryLock around the offending bits of code. For simplicity and to match other parts of the code, I've implemented tryLock. The test from the bug report no longer causes an error on my TSAN build.
PTAL
lgtm https://codereview.chromium.org/1256053006/diff/80001/Source/modules/webaudio... File Source/modules/webaudio/OscillatorNode.cpp (right): https://codereview.chromium.org/1256053006/diff/80001/Source/modules/webaudio... Source/modules/webaudio/OscillatorNode.cpp:230: Is this change necessary? The scope of tryLocker isn't changed.
On 2015/09/11 at 00:02:00, tkent wrote: > lgtm > > https://codereview.chromium.org/1256053006/diff/80001/Source/modules/webaudio... > File Source/modules/webaudio/OscillatorNode.cpp (right): > > https://codereview.chromium.org/1256053006/diff/80001/Source/modules/webaudio... > Source/modules/webaudio/OscillatorNode.cpp:230: > Is this change necessary? The scope of tryLocker isn't changed. I've reverted the change. It's not needed.
The CQ bit was checked by rtoy@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from hongchan@chromium.org, tkent@chromium.org Link to the patchset: https://codereview.chromium.org/1256053006/#ps100001 (title: "Revert change to OscillatorNode.cpp")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1256053006/100001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1256053006/100001
Message was sent while issue was closed.
Committed patchset #6 (id:100001) as https://src.chromium.org/viewvc/blink?view=rev&revision=202152
Message was sent while issue was closed.
Patchset 6 (id:??) landed as https://crrev.com/3165ed4939f10c1768fbfcf8cf894da3b5295d8e |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
