New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Split env library so that there is only one instance of Env::Default()
#43594
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good find. Thank you
This one needs a lot of changes internally since we also have a Google-only |
There is actually an issue with splitting things out like here. External packages using TF Bazel (not from pip package) are supposed to be able to get to |
While working on modular file systems, noticed that tensorflow's pip package includes two copies of `Env::Default()` in two shared objects libraries. One is in `libtensorflow_framework.so`, another is in `_pywrap_tensorflow_internal.so`. Because of that, `Env::Default()` could return different instance depending on when a modular file system is called. The Lookup vs. Register discrepancy may confuse the file system to believe the scheme (e.g., `s3`/`gcs`) is not registered (actually registered by another `Env::Default()`). The duplication of `Env::Default()` is caused by the fact that `tensorflow/core/platform:env` dependency is included in both `libtensorflow_framework.so` and `_pywrap_tensorflow_internal.so`. in bazel indirectly. There are two many dpendency graph component in between `tensorflow/core/platform:env` and `libtensorflow_framework.so`/`_pywrap_tensorflow_internal.so` in bazel. As a result this PR tries to address the issue by split out the `Env::Default()` implementation out and only link it with `libtensorflow_framework.so` to guarantee `libtensorflow_framework.so` is the only place to include `Env::Default()`. Signed-off-by: Yong Tang <yong.tang.github@outlook.com>
…rnal_impl Signed-off-by: Yong Tang <yong.tang.github@outlook.com>
So that //tensorflow/core:lib includes `Env:Default` Signed-off-by: Yong Tang <yong.tang.github@outlook.com>
Thanks @mihaimaruseac for the help. I have made additional changes to the PR and |
PiperOrigin-RevId: 336361140 Change-Id: I9c331eb08b71502c00235a9bad959372c82b46c1
While working on modular file systems, noticed that tensorflow's pip package
includes two copies of
Env::Default()
in two shared objects libraries.One is in
libtensorflow_framework.so
, another is in_pywrap_tensorflow_internal.so
.Because of that,
Env::Default()
could return different instance depending on whena modular file system is called. The Lookup vs. Register discrepancy may confuse the file
system to believe the scheme (e.g.,
s3
/gcs
) is not registered (actually registeredby another
Env::Default()
).The duplication of
Env::Default()
is caused by the fact thattensorflow/core/platform:env
dependency is included in both
libtensorflow_framework.so
and_pywrap_tensorflow_internal.so
.in bazel indirectly.
There are two many dpendency graph component in between
tensorflow/core/platform:env
and
libtensorflow_framework.so
/_pywrap_tensorflow_internal.so
in bazel.As a result this PR tries to address the issue by split out the
Env::Default()
implementationout and only link it with
libtensorflow_framework.so
to guaranteelibtensorflow_framework.so
is the only place to include
Env::Default()
.Signed-off-by: Yong Tang yong.tang.github@outlook.com