search

PixarAnimationStudios / OpenUSD

Universal Scene Description
http://www.openusd.org
Other
5.56k stars 1.15k forks source link

USD includes at least one deprecated or antiquated header #1057

Open sybrenstuvel opened 4 years ago

sybrenstuvel commented 4 years ago

Description of Issue

Building a C++ application (Blender in our case) against the USD libraries on Linux shows the following warning:

This file includes at least one deprecated or antiquated header which may be removed without further notice at a future date

This is caused by the USD library including <ext/hash_set>.

This warning can be silenced by defining _GLIBCXX_PERMIT_BACKWARD_HASH. A proper solution would be to include <unordered_set> and use std::unordered_set<> instead, as this is even recommended by the GNU STD extensions themselves.

Steps to Reproduce

  1. Look at https://github.com/PixarAnimationStudios/USD/blob/master/pxr/base/lib/tf/hashset.h#L38

System Information (OS, Hardware)

Ubuntu Linux 19.04.

Package Versions

GCC 8.3.0

spiffmon commented 4 years ago

Thanks, Sybren. We hope to eventually stop using hash_set. When we open-sourced USD we did a pretty thorough performance analysis and discovered that unordered_map and unordered_set significantly underperformed the gnu hash equivalents in metrics that were important for USD and Presto. So we have kept the older impls as TfHashMap etc.

There are many excellent open-source alternatives now, some of which we’ve been recently exploring. We will definitely address this by the time the hashes disappear - just wanted to let you know its presence is not arbitrary.

On Wed, Dec 11, 2019 at 6:30 AM Sybren A. Stüvel notifications@github.com wrote:

Description of Issue

Building a C++ application (Blender in our case) against the USD libraries on Linux shows the following warning:

This file includes at least one deprecated or antiquated header which may be removed without further notice at a future date

This is caused by the USD library including <ext/hash_set>.

This warning can be silenced by defining _GLIBCXX_PERMIT_BACKWARD_HASH. A proper solution would be to include and use std::unordered_set<> instead, as this is even recommended by the GNU STD extensions https://gcc.gnu.org/onlinedocs/libstdc++/manual/ext_sgi.html themselves. Steps to Reproduce

  1. Look at https://github.com/PixarAnimationStudios/USD/blob/master/pxr/base/lib/tf/hashset.h#L38

System Information (OS, Hardware)

Ubuntu Linux 19.04. Package Versions

GCC 8.3.0

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/PixarAnimationStudios/USD/issues/1057?email_source=notifications&email_token=ABPOU2DVGYHJOD6EBNLJGGDQYD2QRA5CNFSM4JZP4XXKYY3PNVWWK3TUL52HS4DFUVEXG43VMWVGG33NNVSW45C7NFSM4H7Y52BQ, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABPOU2FMCZZ757YAUAFBLXLQYD2QRANCNFSM4JZP4XXA .

-- --spiffiPhone

jilliene commented 4 years ago

Filed as internal issue #USD-5747

syoyo commented 4 years ago

The source of issue is that USD always define ARCH_HAS_GNU_STL_EXTENSIONS for Linux + GCC environemnt

#if defined(ARCH_OS_LINUX) && defined(ARCH_COMPILER_GCC)`
#define ARCH_HAS_GNU_STL_EXTENSIONS
#endif

Here is possible quick fix(add extra check of __cplusplus version)

https://github.com/syoyo/USD-aarch64/commit/7fc57e29bb9d252eb7f1016ccfd8b459f70faf27

For faster unorderd_map and unorderd_set alternatives(C++11 or later),

will be worth to look at.

spiffmon commented 4 years ago

Thank you, Syoyo! We are in fact already investigating the robin map.

On Mon, Dec 23, 2019 at 8:03 AM Syoyo Fujita notifications@github.com wrote:

The source of issue is that USD always define ARCH_HAS_GNU_STL_EXTENSIONS for Linux + GCC environemnt

if defined(ARCH_OS_LINUX) && defined(ARCH_COMPILER_GCC)`

define ARCH_HAS_GNU_STL_EXTENSIONS

endif

Here is possible quick fix(add extra check of __cplusplus version)

syoyo@7fc57e2 https://github.com/syoyo/USD-aarch64/commit/7fc57e29bb9d252eb7f1016ccfd8b459f70faf27

For faster unorderd_map and unorderd_set alternatives(C++11 or later),

will be worth to look at.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/PixarAnimationStudios/USD/issues/1057?email_source=notifications&email_token=ABPOU2HQHMONHGVH4XOZFGTQ2DOLHA5CNFSM4JZP4XXKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEHRNAXY#issuecomment-568512607, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABPOU2EXEIPPXKUWDTCKASLQ2DOLHANCNFSM4JZP4XXA .

-- --spiffiPhone

mistafunk commented 1 year ago

Ping! How did the robin map perform? :-)

This issue actually triggers the last remaining warning in our code base...

spiffmon commented 1 year ago

Hi @mistafunk ! Two answers:

  1. Short-term: if @syoyo or someone else wants to submit a PR for that simple detection fix (gcc 11.x seems to be the first version that does not contain the containers), we would prioritize getting that fix into 23.05, with the understanding that USD performance will be somewhat degraded.
  2. Long-term: We have had good success with the robin-map in a few targeted codesites! One complication is that the majority of the places where we use TfHashMap or TfHashSet it's a performance-sensitive use, and simply swapping out the implementation of the Tf containers wholesale will not be conducive to addressing any fallout. The other complication is that flat-tables like robin-map are not drop-in replacements for bucket-based containers due to reference stability differences. So we need to go through site by site, and there are alot of them. It appears that hash_set/map have been removed in gcc 11.x, which is part of the CY2023 vfxplatform spec; we will shortly be moving USD to CY2022, and to CY2023 sometime in the next year or so. So that provides an upper limit on when we'll need to tackle this work; CY2023 poses several challenges, so it's going to be interesting.
syoyo commented 1 year ago

@spiffmon @mistafunk It took roughly 3 years(!) so I'm not sure the issue still exits. Anyway, if the issue still exists and we want to go with 1. its a very simple fix so it'd be better Pixar internals do it instead of me(I don't mind to submit a PR, but I need to do legal setups if Pixar strictly requires CLA signing even for few lines of code changes 😅)

spiffmon commented 1 year ago

@syoyo - yes, it's a simple fix, but requires more than simple testing for us until we're installing and building with later versions of gcc. But we'll see what we can do - I am positive it is still a problem!

--spiff

On Sat, Dec 17, 2022 at 12:49 PM Syoyo Fujita @.***> wrote:

@spiffmon https://github.com/spiffmon @mistafunk https://github.com/mistafunk It took roughly 3 years(!) so I'm not sure the issue still exits. Anyway, if the issue still exists and we want to go with 1. its a very simple fix so it'd be better Pixar internals do it instead of me(I don't mind to submit a PR, but I need to do legal setups if Pixar strictly requires CLA signing even for few lines of code changes 😅)

— Reply to this email directly, view it on GitHub https://github.com/PixarAnimationStudios/USD/issues/1057#issuecomment-1356462150, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABPOU2B6PZCEOK3XO4YVZYDWNYRMDANCNFSM4JZP4XXA . You are receiving this because you were mentioned.Message ID: @.***>

ikryukov commented 7 months ago

hi, @spiffmon just wanted to add that in addition to hashmap there is TBB warnings: 

[build] In file included from /home/ilya/work/OpenUSD_build/include/pxr/base/work/dispatcher.h:37,
[build]                  from /home/ilya/work/OpenUSD_build/include/pxr/usd/sdf/layer.h:46,
[build]                  from /home/ilya/work/OpenUSD_build/include/pxr/usd/pcp/types.h:30,
[build]                  from /home/ilya/work/OpenUSD_build/include/pxr/usd/pcp/node.h:29,
[build]                  from /home/ilya/work/OpenUSD_build/include/pxr/usd/usd/editTarget.h:31,
[build]                  from /home/ilya/work/OpenUSD_build/include/pxr/usd/usd/stage.h:32,
[build]                  from /home/ilya/work/Strelka/src/hdRunner/main.cpp:24:
[build] /home/ilya/work/OpenUSD_build/include/tbb/task.h:21:139: note: ‘#pragma message: TBB Warning: tbb/task.h is deprecated. For details, please see Deprecated Features appendix in the TBB reference manual.’
[build]    21 | #pragma message("TBB Warning: tbb/task.h is deprecated. For details, please see Deprecated Features appendix in the TBB reference manual.")
[build]       |                                                                                                                                           ^
[build] [ 91%] Building CXX object src
spiffmon commented 7 months ago

Thanks, @ikryukov - I expect the TBB issues to be resolved as we add support for OneTBB in the coming months.

thearperson commented 1 month ago

The source of issue is that USD always define ARCH_HAS_GNU_STL_EXTENSIONS for Linux + GCC environemnt

#if defined(ARCH_OS_LINUX) && defined(ARCH_COMPILER_GCC)`
#define ARCH_HAS_GNU_STL_EXTENSIONS
#endif

Here is possible quick fix(add extra check of __cplusplus version)

syoyo@7fc57e2

For faster unorderd_map and unorderd_set alternatives(C++11 or later),

will be worth to look at.

Just ran into this with our 23.08 based fork (https://github.com/cloud-zeta/OpenUSD). We were using this along with https://github.com/adobe/USD-Fileformat-plugins, which requires add_compile_definitions(_GLIBCXX_USE_CXX11_ABI=1)