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

feat: support stream cmd and lowvram options #157

Open
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

Cyberhan123
Copy link
Contributor

@Cyberhan123 Cyberhan123 commented Jan 23, 2024

This is the draft. Implementing this PR means that we can have greater expansion capabilities in the future, such as easier implementation of CLIP interrogators. At present, this PR has not been completed. I still need to test and complete the logic of the cli. Since the changes are relatively large, I thought I would release them for everyone to see.
cc @leejet @Green-Sky @FSSRepo

@Cyberhan123
Copy link
Contributor Author

There are two IndentCaseLabels in the current .clang-format. I try to set them to true or false, but they will format the code that I have not modified. 😳

@Green-Sky
Copy link
Contributor

Its hard to review with 90% formatting changes.

@Cyberhan123
Copy link
Contributor Author

Its hard to review with 90% formatting changes.

I don’t know much about the formatting method of C++. I used the script in the project to restore it, but with little success. I plan to adjust it manually tomorrow.

for (int i = 0; i < params.batch_count; i++) {
if (results[i].data == NULL) {
continue;
auto instance = new CliInstance(params);
Copy link
Contributor

Choose a reason for hiding this comment

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

dont allocate on the heap. (you should never use new/delete directly, when not absolutely necessary)

CliInstance instance{params};

}
free(results);
free_sd_ctx(sd_ctx);
Copy link
Contributor

Choose a reason for hiding this comment

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

also, dont call exit(), so you can call all the cleanup functions after (free_sd_ctx(), free(), etc)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don’t quite understand what’s wrong with heap allocation. Does it affect performance?My understanding is declared destructor to release the memory automatically.

Copy link
Contributor

Choose a reason for hiding this comment

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

only if freed manually using delete.

@Cyberhan123
Copy link
Contributor Author

Cyberhan123 commented Jan 24, 2024

Thank you for your comment, I will adjust these issues.
When I was writing, I found that manually adjusting the code format while writing was a disaster. I plan to do it when This PR is completed.

--todo reload model
@leejet
Copy link
Owner

leejet commented Jan 29, 2024

I removed the first IndentCaseLabels in the latest master branch, which should be a legacy configuration. I recommend that you use clang-format's command line tool to format your code for consistency.

@Cyberhan123
Copy link
Contributor Author

I removed the first IndentCaseLabels in the latest master branch, which should be a legacy configuration. I recommend that you use clang-format's command line tool to format your code for consistency.

Thank you very much, I don't have time now.

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.

3 participants