Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2684033007:
Allow a ParseInfo without a script for %SetCode users (Closed)
DescriptionThis is a workaround for the fact that %SetCode can "lose" the script for a js native. If the js native is re-initialized (for a Realm or something), then the source SharedFunctionInfo won't have a script anymore. Nonetheless, we may want to optimize the function. If we've compiled bytecode, then we can compile optimized code without a script.
Here, we carve out a special exception for this case, so that we can turn on the --mark-shared-functions-for-tier-up.
BUG=v8:5946
R=leszeks@chromium.org
Review-Url: https://codereview.chromium.org/2684033007
Cr-Original-Commit-Position: refs/heads/master@{#43240}
Committed: https://chromium.googlesource.com/v8/v8/+/4123a3dd790495c40cf839990318a85c146e057d
Review-Url: https://codereview.chromium.org/2684033007
Cr-Commit-Position: refs/heads/master@{#43252}
Committed: https://chromium.googlesource.com/v8/v8/+/ae8f28208f2bd41cee49a4db5a7b5b5b92114842
Patch Set 1 #
Total comments: 1
Patch Set 2 : Make ASAN happy. #Patch Set 3 : Some comments. #
Total comments: 14
Patch Set 4 : Code comments. #
Total comments: 1
Patch Set 5 : REBASE + removed file. #Patch Set 6 : nit #
Messages
Total messages: 74 (55 generated)
Hi Leszek, here is the code we discussed, thx for the look! --Michael
https://codereview.chromium.org/2684033007/diff/1/src/runtime/runtime-functio... File src/runtime/runtime-function.cc (right): https://codereview.chromium.org/2684033007/diff/1/src/runtime/runtime-functio... src/runtime/runtime-function.cc:179: } Hm, I'm not sure I like your solution here -- for one, it doesn't solve the problem that the second %SetCode will clear the script of the target function, so optimising the target function will also fail. Currently this happens to not happen, but if the target function happened to get hot while in stress opt, it could, and some other poor soul will have to spend the day debugging this.
https://codereview.chromium.org/2684033007/diff/1/src/runtime/runtime-functio... File src/runtime/runtime-function.cc (right): https://codereview.chromium.org/2684033007/diff/1/src/runtime/runtime-functio... src/runtime/runtime-function.cc:179: } Hm, I'm not sure I like your solution here -- for one, it doesn't solve the problem that the second %SetCode will clear the script of the target function, so optimising the target function will also fail. Currently this happens to not happen, but if the target function happened to get hot while in stress opt, it could, and some other poor soul will have to spend the day debugging this.
The CQ bit was checked by mvstanton@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Hi Leszek, I've limited my efforts to fixing some locations in the compiler where we expect a script object, though we can live without it. SetCode is doing something fundamentally at odds with the rest of our architecture. It "hands off" a script, failing to realize that we might come back in with the same input functions at realm creation. Experimental natives will always end up with an undefined script in a new realm/context.
Description was changed from ========== Allow %SetCode to work with shared function marking. With shared function marking, and stress-opt, we expose experimental builtins to optimizing compiles. Turn off shared function marking for this scenario. BUG=v8:5946 R=leszeks@chromium.org ========== to ========== Source positions should not crash without a script A bugfix while investigating %SetCode behavior. BUG=v8:5946 R=leszeks@chromium.org ==========
On 2017/02/13 12:04:57, mvstanton wrote: > Hi Leszek, > > I've limited my efforts to fixing some locations in the compiler where we expect > a script object, though we can live without it. > > SetCode is doing something fundamentally at odds with the rest of our > architecture. It "hands off" a script, failing to realize that we might come > back in with the same input functions at realm creation. Experimental natives > will always end up with an undefined script in a new realm/context. Thanks Mike, this is unfortunate but I guess it makes sense. We can revisit when the crunch is less... crunchy. Did you test this with the repros I had in the bug? I think (though haven't checked) that this will still fail if trying to optimise a no-script function because the initialisation of ParseInfo will fail.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by mvstanton@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: v8_win64_rel_ng on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_win64_rel_ng/builds/22469) v8_win64_rel_ng_triggered on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_win64_rel_ng_triggered/b...)
The CQ bit was checked by mvstanton@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: v8_linux64_asan_rel_ng on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_asan_rel_ng/buil...) v8_linux64_avx2_rel_ng on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_avx2_rel_ng/buil...) v8_linux64_gyp_rel_ng on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_gyp_rel_ng/build...) v8_linux64_rel_ng on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_rel_ng/builds/20900) v8_linux64_verify_csa_rel_ng on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_verify_csa_rel_n...) v8_linux_gcc_compile_rel on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux_gcc_compile_rel/bu...) v8_linux_mipsel_compile_rel on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux_mipsel_compile_rel...) v8_presubmit on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_presubmit/builds/34628) v8_win_rel_ng on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_win_rel_ng/builds/22578)
The CQ bit was checked by mvstanton@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: v8_linux64_avx2_rel_ng on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_avx2_rel_ng/buil...) v8_linux64_rel_ng on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_rel_ng/builds/20902)
The CQ bit was checked by mvstanton@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: v8_win_rel_ng on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_win_rel_ng/builds/22633) v8_win_rel_ng_triggered on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_win_rel_ng_triggered/bui...)
The CQ bit was checked by mvstanton@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: v8_win_rel_ng on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_win_rel_ng/builds/22697) v8_win_rel_ng_triggered on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_win_rel_ng_triggered/bui...)
The CQ bit was checked by mvstanton@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
Description was changed from ========== Source positions should not crash without a script A bugfix while investigating %SetCode behavior. BUG=v8:5946 R=leszeks@chromium.org ========== to ========== This is a workaround for the fact that %SetCode can "lose" the script for a js native. If the js native is re-initialized (for a Realm or something), then the source SharedFunctionInfo won't have a script anymore. Nonetheless, we may want to optimize the function. If we've compiled bytecode, then we can compile optimized code without a script. Here, we carve out a special exception for this case, so that we can turn on the --mark-shared-functions-for-tier-up. BUG=v8:5946 R=leszeks@chromium.org ==========
mvstanton@chromium.org changed reviewers: + mstarzinger@chromium.org
Patchset #2 (id:20001) has been deleted
Patchset #2 (id:40001) has been deleted
Patchset #2 (id:60001) has been deleted
Patchset #2 (id:80001) has been deleted
Patchset #2 (id:100001) has been deleted
Patchset #2 (id:120001) has been deleted
Patchset #2 (id:140001) has been deleted
The CQ bit was checked by mvstanton@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Hi Leszek, hi Michi: If you guys could take a look that would be great. Michi, I did my best to localize and call out the exceptional setup of the ParseInfo object, but I wasn't able to move that initialization into the pipeline.cc file (despite a bunch of awkward manipulations with friend functions/classes/etc., ugh). Hopefully things are marked well enough now. This CL now has the flag in question turned on. We could just land it like this if there are no objections. Note that I removed the "tier up" behavior from bytecode to fullcode in GetLazyCode(), per my conversations with Leszek and Michi. All the best, --Michael
LGTM once comments are addressed. https://codereview.chromium.org/2684033007/diff/180001/src/compiler/pipeline.cc File src/compiler/pipeline.cc (right): https://codereview.chromium.org/2684033007/diff/180001/src/compiler/pipeline.... src/compiler/pipeline.cc:1755: Isolate* isolate = function->shared()->GetIsolate(); nit: The isolate is only used for the handle allocation below the work involved is exactly the same is if were were to just use the general handle constructor, but it's easier to read: Handle<SharedFunctionInfo> shared(function->shared()); https://codereview.chromium.org/2684033007/diff/180001/src/compiler/pipeline.... src/compiler/pipeline.cc:1757: ParseInfo* p; nit: s/p/parse_info/ https://codereview.chromium.org/2684033007/diff/180001/src/flag-definitions.h File src/flag-definitions.h (right): https://codereview.chromium.org/2684033007/diff/180001/src/flag-definitions.h... src/flag-definitions.h:272: DEFINE_BOOL(mark_shared_functions_for_tier_up, true, Should we land the flag switch as part of this CL or separate? If we do it separate it is easier to just revert one but not the other. https://codereview.chromium.org/2684033007/diff/180001/src/parsing/parse-info.cc File src/parsing/parse-info.cc (right): https://codereview.chromium.org/2684033007/diff/180001/src/parsing/parse-info... src/parsing/parse-info.cc:62: if (shared->script()->IsScript()) { This should no longer be needed since we have the dedicated static function below. Can we undo this part of the change? https://codereview.chromium.org/2684033007/diff/180001/src/source-position.cc File src/source-position.cc (right): https://codereview.chromium.org/2684033007/diff/180001/src/source-position.cc... src/source-position.cc:95: if (source_name->IsString()) { nit: It is kind of scary that we treat the non-Script case as Smi(0) here. Can we change to to "source_name != nullptr" similar to how we do above?
lgtm with nits. https://codereview.chromium.org/2684033007/diff/180001/src/compiler.cc File src/compiler.cc (right): https://codereview.chromium.org/2684033007/diff/180001/src/compiler.cc#newcod... src/compiler.cc:1076: case Compiler::BASELINE: { Can you add a comment here stating that we don't tier up to BASELINE because GetBaselineCode doesn't support top-level/eval code? https://codereview.chromium.org/2684033007/diff/180001/test/mjsunit/shared-fu... File test/mjsunit/shared-function-tier-up-ignition.js (right): https://codereview.chromium.org/2684033007/diff/180001/test/mjsunit/shared-fu... test/mjsunit/shared-function-tier-up-ignition.js:27: %OptimizeFunctionOnNextCall(f); Let's move this and the below to i==1 and i==2, and get rid of 2 iterations. See the other tests, shared-function-tier-up-default.js and shared-function-tier-up-turbo.js
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
thanks guys! --Michael https://codereview.chromium.org/2684033007/diff/180001/src/compiler.cc File src/compiler.cc (right): https://codereview.chromium.org/2684033007/diff/180001/src/compiler.cc#newcod... src/compiler.cc:1076: case Compiler::BASELINE: { On 2017/02/15 14:58:37, Leszek Swirski wrote: > Can you add a comment here stating that we don't tier up to BASELINE because > GetBaselineCode doesn't support top-level/eval code? Yep, done. https://codereview.chromium.org/2684033007/diff/180001/src/compiler/pipeline.cc File src/compiler/pipeline.cc (right): https://codereview.chromium.org/2684033007/diff/180001/src/compiler/pipeline.... src/compiler/pipeline.cc:1755: Isolate* isolate = function->shared()->GetIsolate(); On 2017/02/15 14:51:48, Michael Starzinger wrote: > nit: The isolate is only used for the handle allocation below the work involved > is exactly the same is if were were to just use the general handle constructor, > but it's easier to read: > > Handle<SharedFunctionInfo> shared(function->shared()); Done. https://codereview.chromium.org/2684033007/diff/180001/src/compiler/pipeline.... src/compiler/pipeline.cc:1757: ParseInfo* p; On 2017/02/15 14:51:48, Michael Starzinger wrote: > nit: s/p/parse_info/ Done. https://codereview.chromium.org/2684033007/diff/180001/src/flag-definitions.h File src/flag-definitions.h (right): https://codereview.chromium.org/2684033007/diff/180001/src/flag-definitions.h... src/flag-definitions.h:272: DEFINE_BOOL(mark_shared_functions_for_tier_up, true, On 2017/02/15 14:51:48, Michael Starzinger wrote: > Should we land the flag switch as part of this CL or separate? If we do it > separate it is easier to just revert one but not the other. Yep, I'll turn it off in this CL now. https://codereview.chromium.org/2684033007/diff/180001/src/parsing/parse-info.cc File src/parsing/parse-info.cc (right): https://codereview.chromium.org/2684033007/diff/180001/src/parsing/parse-info... src/parsing/parse-info.cc:62: if (shared->script()->IsScript()) { On 2017/02/15 14:51:48, Michael Starzinger wrote: > This should no longer be needed since we have the dedicated static function > below. Can we undo this part of the change? doh! Done. https://codereview.chromium.org/2684033007/diff/180001/src/source-position.cc File src/source-position.cc (right): https://codereview.chromium.org/2684033007/diff/180001/src/source-position.cc... src/source-position.cc:95: if (source_name->IsString()) { On 2017/02/15 14:51:48, Michael Starzinger wrote: > nit: It is kind of scary that we treat the non-Script case as Smi(0) here. Can > we change to to "source_name != nullptr" similar to how we do above? Indeed! Done. https://codereview.chromium.org/2684033007/diff/180001/test/mjsunit/shared-fu... File test/mjsunit/shared-function-tier-up-ignition.js (right): https://codereview.chromium.org/2684033007/diff/180001/test/mjsunit/shared-fu... test/mjsunit/shared-function-tier-up-ignition.js:27: %OptimizeFunctionOnNextCall(f); On 2017/02/15 14:58:37, Leszek Swirski wrote: > Let's move this and the below to i==1 and i==2, and get rid of 2 iterations. See > the other tests, shared-function-tier-up-default.js and > shared-function-tier-up-turbo.js Done.
The CQ bit was checked by mvstanton@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from mstarzinger@chromium.org, leszeks@chromium.org Link to the patchset: https://codereview.chromium.org/2684033007/#ps200001 (title: "Code comments.")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: v8_presubmit on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_presubmit/builds/34828)
https://codereview.chromium.org/2684033007/diff/200001/test/mjsunit/shared-fu... File test/mjsunit/shared-function-tier-up-ignition.js (right): https://codereview.chromium.org/2684033007/diff/200001/test/mjsunit/shared-fu... test/mjsunit/shared-function-tier-up-ignition.js:6: // Flags: --ignition-staging --turbo I just noticed this -- if we're fixing this to --turbo then we may as well get rid of it entirely, there's another test for turbo IIRC
mvstanton@chromium.org changed reviewers: + marja@chromium.org
Hi Marja, Here are the changes we talked about a couple days ago. Could you have a look at parsing files? Thanks, --Michael
src/parsing lgtm
The CQ bit was checked by mvstanton@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from leszeks@chromium.org, mstarzinger@chromium.org, marja@chromium.org Link to the patchset: https://codereview.chromium.org/2684033007/#ps220001 (title: "REBASE + removed file.")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 220001, "attempt_start_ts": 1487238683949130,
"parent_rev": "d31c5410c4fdfc5eb66582892d5e3ecd3706bd58", "commit_rev":
"4123a3dd790495c40cf839990318a85c146e057d"}
Message was sent while issue was closed.
Description was changed from ========== This is a workaround for the fact that %SetCode can "lose" the script for a js native. If the js native is re-initialized (for a Realm or something), then the source SharedFunctionInfo won't have a script anymore. Nonetheless, we may want to optimize the function. If we've compiled bytecode, then we can compile optimized code without a script. Here, we carve out a special exception for this case, so that we can turn on the --mark-shared-functions-for-tier-up. BUG=v8:5946 R=leszeks@chromium.org ========== to ========== This is a workaround for the fact that %SetCode can "lose" the script for a js native. If the js native is re-initialized (for a Realm or something), then the source SharedFunctionInfo won't have a script anymore. Nonetheless, we may want to optimize the function. If we've compiled bytecode, then we can compile optimized code without a script. Here, we carve out a special exception for this case, so that we can turn on the --mark-shared-functions-for-tier-up. BUG=v8:5946 R=leszeks@chromium.org Review-Url: https://codereview.chromium.org/2684033007 Cr-Commit-Position: refs/heads/master@{#43240} Committed: https://chromium.googlesource.com/v8/v8/+/4123a3dd790495c40cf839990318a85c146... ==========
Message was sent while issue was closed.
Committed patchset #5 (id:220001) as https://chromium.googlesource.com/v8/v8/+/4123a3dd790495c40cf839990318a85c146...
Message was sent while issue was closed.
A revert of this CL (patchset #5 id:220001) has been created in https://codereview.chromium.org/2703553002/ by machenbach@chromium.org. The reason for reverting is: Please remove the file in status file too. Breaks presubmit: https://build.chromium.org/p/client.v8/builders/V8%20Linux%20-%20presubmit/bu... Or lets call it post-submit :(.
Message was sent while issue was closed.
Description was changed from ========== This is a workaround for the fact that %SetCode can "lose" the script for a js native. If the js native is re-initialized (for a Realm or something), then the source SharedFunctionInfo won't have a script anymore. Nonetheless, we may want to optimize the function. If we've compiled bytecode, then we can compile optimized code without a script. Here, we carve out a special exception for this case, so that we can turn on the --mark-shared-functions-for-tier-up. BUG=v8:5946 R=leszeks@chromium.org Review-Url: https://codereview.chromium.org/2684033007 Cr-Commit-Position: refs/heads/master@{#43240} Committed: https://chromium.googlesource.com/v8/v8/+/4123a3dd790495c40cf839990318a85c146... ========== to ========== This is a workaround for the fact that %SetCode can "lose" the script for a js native. If the js native is re-initialized (for a Realm or something), then the source SharedFunctionInfo won't have a script anymore. Nonetheless, we may want to optimize the function. If we've compiled bytecode, then we can compile optimized code without a script. Here, we carve out a special exception for this case, so that we can turn on the --mark-shared-functions-for-tier-up. BUG=v8:5946 R=leszeks@chromium.org Review-Url: https://codereview.chromium.org/2684033007 Cr-Commit-Position: refs/heads/master@{#43240} Committed: https://chromium.googlesource.com/v8/v8/+/4123a3dd790495c40cf839990318a85c146... ==========
The CQ bit was checked by mvstanton@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from leszeks@chromium.org, mstarzinger@chromium.org, marja@chromium.org Link to the patchset: https://codereview.chromium.org/2684033007/#ps240001 (title: "nit")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 240001, "attempt_start_ts": 1487254506387170,
"parent_rev": "4b0edcf7e06169b6daf0c18c14c42265a3024006", "commit_rev":
"ae8f28208f2bd41cee49a4db5a7b5b5b92114842"}
Message was sent while issue was closed.
Description was changed from ========== This is a workaround for the fact that %SetCode can "lose" the script for a js native. If the js native is re-initialized (for a Realm or something), then the source SharedFunctionInfo won't have a script anymore. Nonetheless, we may want to optimize the function. If we've compiled bytecode, then we can compile optimized code without a script. Here, we carve out a special exception for this case, so that we can turn on the --mark-shared-functions-for-tier-up. BUG=v8:5946 R=leszeks@chromium.org Review-Url: https://codereview.chromium.org/2684033007 Cr-Commit-Position: refs/heads/master@{#43240} Committed: https://chromium.googlesource.com/v8/v8/+/4123a3dd790495c40cf839990318a85c146... ========== to ========== This is a workaround for the fact that %SetCode can "lose" the script for a js native. If the js native is re-initialized (for a Realm or something), then the source SharedFunctionInfo won't have a script anymore. Nonetheless, we may want to optimize the function. If we've compiled bytecode, then we can compile optimized code without a script. Here, we carve out a special exception for this case, so that we can turn on the --mark-shared-functions-for-tier-up. BUG=v8:5946 R=leszeks@chromium.org Review-Url: https://codereview.chromium.org/2684033007 Cr-Original-Commit-Position: refs/heads/master@{#43240} Committed: https://chromium.googlesource.com/v8/v8/+/4123a3dd790495c40cf839990318a85c146... Review-Url: https://codereview.chromium.org/2684033007 Cr-Commit-Position: refs/heads/master@{#43252} Committed: https://chromium.googlesource.com/v8/v8/+/ae8f28208f2bd41cee49a4db5a7b5b5b921... ==========
Message was sent while issue was closed.
Committed patchset #6 (id:240001) as https://chromium.googlesource.com/v8/v8/+/ae8f28208f2bd41cee49a4db5a7b5b5b921... |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
