Skip to content

Conversation

dskkato
Copy link
Contributor

@dskkato dskkato commented Jun 1, 2021

When I compare TString usage in cppflow and tensorflow's unittest, it seems that deallocation step may be required. But I did not check actual implementation of TString, so if I am wrong, please let it go.

https://github.com/tensorflow/tensorflow/blob/2305f9d0a58921987788fbd1d8fa43eda888233f/tensorflow/core/platform/ctstring_test.cc#L34-L68

@ljn917
Copy link
Contributor

ljn917 commented Sep 28, 2021

I believe this should be merged, though not tested. Thanks.

In addition, we also need to add test cases for the string typed tensor.

@serizba serizba force-pushed the master branch 2 times, most recently from 3326662 to c98a475 Compare February 28, 2022 10:20
Copy link

@ns-wxin ns-wxin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My simple test doesn't complain memory leak any more. However, when running against TF_TString {1} model, I got "heap-use-after-free". I can share with you my model if you would like to try it yourself.

The following code doesn't complain memory leak anymore.

int main(int argc, char **argv) {
    // string modelPath = argv[1];
    string sent1 {"I enjoy taking long walks\n along the beach with my dog."};
    string sent2 {"I taking long walks\n along the beach with my dog."};

    // auto sentence = cppflow::tensor(text);
    auto sentence = cppflow::tensor(sent1);
    auto sentence2 = cppflow::tensor(sent2);
    std::cout << "tensor: " << sentence << std::endl;
    // vector<TF_TString> data = sentence.get_data<TF_TString>();

    // cppflow::model model(modelPath);

    return 0;
}

However, the following code caused the "heap-use-after-free".
"tensor.h":

    template<>
    inline tensor::tensor(const std::string& value) {
        TF_TString tstr[1];
        TF_TString_Init(&tstr[0]);
        TF_TString_Copy(&tstr[0], value.c_str(), value.size());

        // *this = tensor(static_cast<enum TF_DataType>(TF_STRING), (void *) tstr, sizeof(tstr), {});

        *this = tensor(static_cast<enum TF_DataType>(TF_STRING), (void *) tstr, sizeof(tstr), {1});
        TF_TString_Dealloc(&tstr[0]);
    }

my test code:

vector<float> generateEmbedding(cppflow::model model, string text) {
    // vector<string> textVec(1);
    // textVec.push_back(text);
    cppflow::tensor sentence = cppflow::tensor(text);
    // cppflow::tensor sentence = cppflow::tensor(textVec);
    std::vector<cppflow::tensor> output = model({{"serving_default_sentences:0", sentence}},{"StatefulPartitionedCall_2:0"});
    // vector<TF_TString> rawData = sentence.get_data<TF_TString>();
    
    return convertModelResult(output);
}

@serizba
Copy link
Owner

serizba commented Sep 23, 2022

This solution does not work, as it deallocates before using the tensor.

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.

4 participants