Skip to content
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

Logging #345

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from
Draft

Logging #345

wants to merge 3 commits into from

Conversation

rnett
Copy link
Contributor

@rnett rnett commented Jun 25, 2021

This PR is mapping TFLogSink from tensorflow/tensorflow#41733 (which replaces TF_RegisterLogListener) and hooking it up to Slf4j.

@rnett
Copy link
Contributor Author

rnett commented Jun 25, 2021

@saudet I'm getting c++ errors when mapping absl::string_view. I used the same info as you did in the tensorflow preset:

.put(new Info("absl::string_view", "absl::lts_2020_09_23::string_view", "string", "std::string", "tensorflow::string").annotations("@StdString")
            .valueTypes("@Cast({\"char*\", \"std::string&&\"}) BytePointer", "@Cast({\"char*\", \"std::string&&\"}) String")
            .pointerTypes("@Cast({\"char*\", \"std::string*\"}) BytePointer"))

but I'm getting errors:

/windows/Users/jimne/Desktop/OtherStuff/tensorflow_java/tensorflow-core/tensorflow-core-api/target/native/org/tensorflow/internal/c_api/linux-x86_64/jnitensorflow.cpp: In function ‘_jobject* Java_org_tensorflow_internal_c_1api_TFLogEntry_text_1message(JNIEnv*, jobject)’:
/windows/Users/jimne/Desktop/OtherStuff/tensorflow_java/tensorflow-core/tensorflow-core-api/target/native/org/tensorflow/internal/c_api/linux-x86_64/jnitensorflow.cpp:2465:59: error: no matching function for call to ‘StringAdapter<char>::StringAdapter(absl::lts_2020_09_23::string_view)’
         StringAdapter< char > radapter(ptr->text_message());
                                                           ^
/windows/Users/jimne/Desktop/OtherStuff/tensorflow_java/tensorflow-core/tensorflow-core-api/target/native/org/tensorflow/internal/c_api/linux-x86_64/jnitensorflow.cpp:461:5: note: candidate: StringAdapter<T>::StringAdapter(const std::__cxx11::basic_string<_CharT>*) [with T = char]
     StringAdapter(const std::basic_string<T>* str) : ptr(0), size(0), owner(0), str(*(std::basic_string<T>*)str) { }
     ^~~~~~~~~~~~~
/windows/Users/jimne/Desktop/OtherStuff/tensorflow_java/tensorflow-core/tensorflow-core-api/target/native/org/tensorflow/internal/c_api/linux-x86_64/jnitensorflow.cpp:461:5: note:   no known conversion for argument 1 from ‘absl::lts_2020_09_23::string_view’ to ‘const std::__cxx11::basic_string<char>*’
/windows/Users/jimne/Desktop/OtherStuff/tensorflow_java/tensorflow-core/tensorflow-core-api/target/native/org/tensorflow/internal/c_api/linux-x86_64/jnitensorflow.cpp:460:5: note: candidate: StringAdapter<T>::StringAdapter(std::__cxx11::basic_string<_CharT>&) [with T = char]
     StringAdapter(      std::basic_string<T>& str) : ptr(0), size(0), owner(0), str(str) { }
     ^~~~~~~~~~~~~
/windows/Users/jimne/Desktop/OtherStuff/tensorflow_java/tensorflow-core/tensorflow-core-api/target/native/org/tensorflow/internal/c_api/linux-x86_64/jnitensorflow.cpp:460:5: note:   no known conversion for argument 1 from ‘absl::lts_2020_09_23::string_view’ to ‘std::__cxx11::basic_string<char>&’
/windows/Users/jimne/Desktop/OtherStuff/tensorflow_java/tensorflow-core/tensorflow-core-api/target/native/org/tensorflow/internal/c_api/linux-x86_64/jnitensorflow.cpp:459:5: note: candidate: StringAdapter<T>::StringAdapter(const std::__cxx11::basic_string<_CharT>&) [with T = char]
     StringAdapter(const std::basic_string<T>& str) : ptr(0), size(0), owner(0), str2(str), str(str2) { }
     ^~~~~~~~~~~~~
/windows/Users/jimne/Desktop/OtherStuff/tensorflow_java/tensorflow-core/tensorflow-core-api/target/native/org/tensorflow/internal/c_api/linux-x86_64/jnitensorflow.cpp:459:5: note:   no known conversion for argument 1 from ‘absl::lts_2020_09_23::string_view’ to ‘const std::__cxx11::basic_string<char>&’
/windows/Users/jimne/Desktop/OtherStuff/tensorflow_java/tensorflow-core/tensorflow-core-api/target/native/org/tensorflow/internal/c_api/linux-x86_64/jnitensorflow.cpp:457:5: note: candidate: StringAdapter<T>::StringAdapter(const int*, typename std::__cxx11::basic_string<_CharT>::size_type, void*) [with T = char; typename std::__cxx11::basic_string<_CharT>::size_type = long unsigned int]
     StringAdapter(const   signed   int* ptr, typename std::basic_string<T>::size_type size, void* owner) : ptr((T*)ptr), size(size), owner(owner),
     ^~~~~~~~~~~~~
/windows/Users/jimne/Desktop/OtherStuff/tensorflow_java/tensorflow-core/tensorflow-core-api/target/native/org/tensorflow/internal/c_api/linux-x86_64/jnitensorflow.cpp:457:5: note:   candidate expects 3 arguments, 1 provided
/windows/Users/jimne/Desktop/OtherStuff/tensorflow_java/tensorflow-core/tensorflow-core-api/target/native/org/tensorflow/internal/c_api/linux-x86_64/jnitensorflow.cpp:455:5: note: candidate: StringAdapter<T>::StringAdapter(const short unsigned int*, typename std::__cxx11::basic_string<_CharT>::size_type, void*) [with T = char; typename std::__cxx11::basic_string<_CharT>::size_type = long unsigned int]
     StringAdapter(const unsigned short* ptr, typename std::basic_string<T>::size_type size, void* owner) : ptr((T*)ptr), size(size), owner(owner),
     ^~~~~~~~~~~~~
/windows/Users/jimne/Desktop/OtherStuff/tensorflow_java/tensorflow-core/tensorflow-core-api/target/native/org/tensorflow/internal/c_api/linux-x86_64/jnitensorflow.cpp:455:5: note:   candidate expects 3 arguments, 1 provided
/windows/Users/jimne/Desktop/OtherStuff/tensorflow_java/tensorflow-core/tensorflow-core-api/target/native/org/tensorflow/internal/c_api/linux-x86_64/jnitensorflow.cpp:453:5: note: candidate: StringAdapter<T>::StringAdapter(const wchar_t*, typename std::__cxx11::basic_string<_CharT>::size_type, void*) [with T = char; typename std::__cxx11::basic_string<_CharT>::size_type = long unsigned int]
     StringAdapter(const       wchar_t* ptr, typename std::basic_string<T>::size_type size, void* owner) : ptr((T*)ptr), size(size), owner(owner),
     ^~~~~~~~~~~~~
/windows/Users/jimne/Desktop/OtherStuff/tensorflow_java/tensorflow-core/tensorflow-core-api/target/native/org/tensorflow/internal/c_api/linux-x86_64/jnitensorflow.cpp:453:5: note:   candidate expects 3 arguments, 1 provided
/windows/Users/jimne/Desktop/OtherStuff/tensorflow_java/tensorflow-core/tensorflow-core-api/target/native/org/tensorflow/internal/c_api/linux-x86_64/jnitensorflow.cpp:451:5: note: candidate: StringAdapter<T>::StringAdapter(const unsigned char*, typename std::__cxx11::basic_string<_CharT>::size_type, void*) [with T = char; typename std::__cxx11::basic_string<_CharT>::size_type = long unsigned int]
     StringAdapter(const unsigned char* ptr, typename std::basic_string<T>::size_type size, void* owner) : ptr((T*)ptr), size(size), owner(owner),
     ^~~~~~~~~~~~~
/windows/Users/jimne/Desktop/OtherStuff/tensorflow_java/tensorflow-core/tensorflow-core-api/target/native/org/tensorflow/internal/c_api/linux-x86_64/jnitensorflow.cpp:451:5: note:   candidate expects 3 arguments, 1 provided
/windows/Users/jimne/Desktop/OtherStuff/tensorflow_java/tensorflow-core/tensorflow-core-api/target/native/org/tensorflow/internal/c_api/linux-x86_64/jnitensorflow.cpp:449:5: note: candidate: StringAdapter<T>::StringAdapter(const signed char*, typename std::__cxx11::basic_string<_CharT>::size_type, void*) [with T = char; typename std::__cxx11::basic_string<_CharT>::size_type = long unsigned int]
     StringAdapter(const signed   char* ptr, typename std::basic_string<T>::size_type size, void* owner) : ptr((T*)ptr), size(size), owner(owner),
     ^~~~~~~~~~~~~
/windows/Users/jimne/Desktop/OtherStuff/tensorflow_java/tensorflow-core/tensorflow-core-api/target/native/org/tensorflow/internal/c_api/linux-x86_64/jnitensorflow.cpp:449:5: note:   candidate expects 3 arguments, 1 provided
/windows/Users/jimne/Desktop/OtherStuff/tensorflow_java/tensorflow-core/tensorflow-core-api/target/native/org/tensorflow/internal/c_api/linux-x86_64/jnitensorflow.cpp:447:5: note: candidate: StringAdapter<T>::StringAdapter(const char*, typename std::__cxx11::basic_string<_CharT>::size_type, void*) [with T = char; typename std::__cxx11::basic_string<_CharT>::size_type = long unsigned int]
     StringAdapter(const          char* ptr, typename std::basic_string<T>::size_type size, void* owner) : ptr((T*)ptr), size(size), owner(owner),
     ^~~~~~~~~~~~~
/windows/Users/jimne/Desktop/OtherStuff/tensorflow_java/tensorflow-core/tensorflow-core-api/target/native/org/tensorflow/internal/c_api/linux-x86_64/jnitensorflow.cpp:447:5: note:   candidate expects 3 arguments, 1 provided
/windows/Users/jimne/Desktop/OtherStuff/tensorflow_java/tensorflow-core/tensorflow-core-api/target/native/org/tensorflow/internal/c_api/linux-x86_64/jnitensorflow.cpp:445:50: note: candidate: StringAdapter<char>::StringAdapter(const StringAdapter<char>&)
 template<typename T = char> class JavaCPP_hidden StringAdapter {
                                                  ^~~~~~~~~~~~~
/windows/Users/jimne/Desktop/OtherStuff/tensorflow_java/tensorflow-core/tensorflow-core-api/target/native/org/tensorflow/internal/c_api/linux-x86_64/jnitensorflow.cpp:445:50: note:   no known conversion for argument 1 from ‘absl::lts_2020_09_23::string_view’ to ‘const StringAdapter<char>&’
/windows/Users/jimne/Desktop/OtherStuff/tensorflow_java/tensorflow-core/tensorflow-core-api/target/native/org/tensorflow/internal/c_api/linux-x86_64/jnitensorflow.cpp:445:50: note: candidate: StringAdapter<char>::StringAdapter(StringAdapter<char>&&)
/windows/Users/jimne/Desktop/OtherStuff/tensorflow_java/tensorflow-core/tensorflow-core-api/target/native/org/tensorflow/internal/c_api/linux-x86_64/jnitensorflow.cpp:445:50: note:   no known conversion for argument 1 from ‘absl::lts_2020_09_23::string_view’ to ‘StringAdapter<char>&&’

Any idea why?

@rnett rnett marked this pull request as draft June 25, 2021 19:06
@saudet
Copy link
Contributor

saudet commented Jun 25, 2021

string_view doesn't provide an implicit cast to std::string, so we have to cast it explicitly. I'm not too sure what we should do about that. I've currently mapped those to PointerPointer elsewhere since it didn't look like functions that were useful anyway, but it's probably not what you want to do here.

@rnett
Copy link
Contributor Author

rnett commented Jun 26, 2021

All I need to do is read the string from it, (i.e. TFLogEntry.text_message()), is there a way to use PointerPointer and do that cast manually?

saudet added a commit to bytedeco/javacpp that referenced this pull request Jun 26, 2021
@saudet
Copy link
Contributor

saudet commented Jun 26, 2021

I've made it possible to apply a cast there in commit bytedeco/javacpp@3feb62d. In this case, something like
"@Cast({"char*", "std::string&&", "std::string"}) or possibly just "@Cast({"", "", "std::string"}) should work.

@rnett
Copy link
Contributor Author

rnett commented Jun 27, 2021

Getting new errors like:

                                                                                     ^
/windows/Users/jimne/Desktop/OtherStuff/tensorflow_java/tensorflow-core/tensorflow-core-api/target/native/org/tensorflow/internal/c_api/linux-x86_64/jnitensorflow.cpp: In function ‘void Java_org_tensorflow_internal_c_1api_TFLogEntry_allocate__ILorg_bytedeco_javacpp_BytePointer_2(JNIEnv*, jobject, jint, jobject)’:
/windows/Users/jimne/Desktop/OtherStuff/tensorflow_java/tensorflow-core/tensorflow-core-api/target/native/org/tensorflow/internal/c_api/linux-x86_64/jnitensorflow.cpp:2173:97: error: call of overloaded ‘basic_string(StringAdapter<char>&)’ is ambiguous
     tensorflow::TFLogEntry* rptr = new (std::nothrow) tensorflow::TFLogEntry(arg0, (std::string)adapter1);
                                                                                                 ^~~~~~~~
In file included from /usr/include/c++/7/string:52:0,
                 from /usr/include/c++/7/stdexcept:39,
                 from /usr/include/c++/7/array:39,
                 from /usr/include/c++/7/tuple:39,
                 from /usr/include/c++/7/bits/unique_ptr.h:37,
                 from /usr/include/c++/7/memory:80,
                 from /windows/Users/jimne/Desktop/OtherStuff/tensorflow_java/tensorflow-core/tensorflow-core-api/target/native/org/tensorflow/internal/c_api/linux-x86_64/jnitensorflow.cpp:77:
/usr/include/c++/7/bits/basic_string.h:509:7: note: candidate: std::__cxx11::basic_string<_CharT, _Traits, _Alloc>::basic_string(const _CharT*, const _Alloc&) [with _CharT = char; _Traits = std::char_traits<char>; _Alloc = std::allocator<char>]
       basic_string(const _CharT* __s, const _Alloc& __a = _Alloc())
       ^~~~~~~~~~~~
/usr/include/c++/7/bits/basic_string.h:437:7: note: candidate: std::__cxx11::basic_string<_CharT, _Traits, _Alloc>::basic_string(const std::__cxx11::basic_string<_CharT, _Traits, _Alloc>&) [with _CharT = char; _Traits = std::char_traits<char>; _Alloc = std::allocator<char>]
       basic_string(const basic_string& __str)
       ^~~~~~~~~~~~
/windows/Users/jimne/Desktop/OtherStuff/tensorflow_java/tensorflow-core/tensorflow-core-api/target/native/org/tensorflow/internal/c_api/linux-x86_64/jnitensorflow.cpp: In function ‘void Java_org_tensorflow_internal_c_1api_TFLogEntry_allocate__ILjava_lang_String_2ILjava_lang_String_2(JNIEnv*, jobject, jint, jstring, jint, jstring)’:
/windows/Users/jimne/Desktop/OtherStuff/tensorflow_java/tensorflow-core/tensorflow-core-api/target/native/org/tensorflow/internal/c_api/linux-x86_64/jnitensorflow.cpp:2194:97: error: call of overloaded ‘basic_string(StringAdapter<char>&)’ is ambiguous
     tensorflow::TFLogEntry* rptr = new (std::nothrow) tensorflow::TFLogEntry(arg0, (std::string)adapter1, arg2, (std::string)adapter3);
                                                                                                 ^~~~~~~~
In file included from /usr/include/c++/7/string:52:0,
                 from /usr/include/c++/7/stdexcept:39,
                 from /usr/include/c++/7/array:39,
                 from /usr/include/c++/7/tuple:39,
                 from /usr/include/c++/7/bits/unique_ptr.h:37,
                 from /usr/include/c++/7/memory:80,
                 from /windows/Users/jimne/Desktop/OtherStuff/tensorflow_java/tensorflow-core/tensorflow-core-api/target/native/org/tensorflow/internal/c_api/linux-x86_64/jnitensorflow.cpp:77:
/usr/include/c++/7/bits/basic_string.h:509:7: note: candidate: std::__cxx11::basic_string<_CharT, _Traits, _Alloc>::basic_string(const _CharT*, const _Alloc&) [with _CharT = char; _Traits = std::char_traits<char>; _Alloc = std::allocator<char>]
       basic_string(const _CharT* __s, const _Alloc& __a = _Alloc())
       ^~~~~~~~~~~~
/usr/include/c++/7/bits/basic_string.h:437:7: note: candidate: std::__cxx11::basic_string<_CharT, _Traits, _Alloc>::basic_string(const std::__cxx11::basic_string<_CharT, _Traits, _Alloc>&) [with _CharT = char; _Traits = std::char_traits<char>; _Alloc = std::allocator<char>]
       basic_string(const basic_string& __str)
       ^~~~~~~~~~~~
/windows/Users/jimne/Desktop/OtherStuff/tensorflow_java/tensorflow-core/tensorflow-core-api/target/native/org/tensorflow/internal/c_api/linux-x86_64/jnitensorflow.cpp:2194:126: error: call of overloaded ‘basic_string(StringAdapter<char>&)’ is ambiguous
     tensorflow::TFLogEntry* rptr = new (std::nothrow) tensorflow::TFLogEntry(arg0, (std::string)adapter1, arg2, (std::string)adapter3);
                                                                                                                              ^~~~~~~~
In file included from /usr/include/c++/7/string:52:0,
                 from /usr/include/c++/7/stdexcept:39,
                 from /usr/include/c++/7/array:39,
                 from /usr/include/c++/7/tuple:39,
                 from /usr/include/c++/7/bits/unique_ptr.h:37,
                 from /usr/include/c++/7/memory:80,
                 from /windows/Users/jimne/Desktop/OtherStuff/tensorflow_java/tensorflow-core/tensorflow-core-api/target/native/org/tensorflow/internal/c_api/linux-x86_64/jnitensorflow.cpp:77:
/usr/include/c++/7/bits/basic_string.h:509:7: note: candidate: std::__cxx11::basic_string<_CharT, _Traits, _Alloc>::basic_string(const _CharT*, const _Alloc&) [with _CharT = char; _Traits = std::char_traits<char>; _Alloc = std::allocator<char>]
       basic_string(const _CharT* __s, const _Alloc& __a = _Alloc())
       ^~~~~~~~~~~~
/usr/include/c++/7/bits/basic_string.h:437:7: note: candidate: std::__cxx11::basic_string<_CharT, _Traits, _Alloc>::basic_string(const std::__cxx11::basic_string<_CharT, _Traits, _Alloc>&) [with _CharT = char; _Traits = std::char_traits<char>; _Alloc = std::allocator<char>]
       basic_string(const basic_string& __str)
       ^~~~~~~~~~~~

after I changed the info to:

.put(
            new Info("absl::string_view")
                .annotations("@StdString")
                .valueTypes(
                    "@Cast({\"\", \"\", \"std::string\"}) BytePointer",
                    "@Cast({\"\", \"\", \"std::string\"}) String")
                .pointerTypes("@Cast({\"\", \"\", \"std::string\"}) BytePointer"))

@rnett
Copy link
Contributor Author

rnett commented Jun 27, 2021

I managed to work around this by purifying the generated class, turns out the only string_views are in the constructors which I don't think will be needed. May come up later, but we'll see.

Well, all except the text message, which I need.

Signed-off-by: Ryan Nett <[email protected]>
Signed-off-by: Ryan Nett <[email protected]>
@saudet
Copy link
Contributor

saudet commented Jun 27, 2021

Just do "@Cast({"char*", "std::string&&", "std::string"}) that should work then.

Signed-off-by: Ryan Nett <[email protected]>
@rnett
Copy link
Contributor Author

rnett commented Jun 27, 2021

Ok, the prototype works, but it doesn't get all log messages. Reported at tensorflow/tensorflow#44995 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants