search

RainEggplant / chatgpt-telegram-bot

A ChatGPT bot for Telegram based on Node.js. Support both browserless and browser-base APIs.
MIT License
323 stars 97 forks source link

Added queue implementation #19

Closed Vadko closed 1 year ago

Vadko commented 1 year ago

https://user-images.githubusercontent.com/9297785/221294664-11e866f6-a415-4614-a5ae-3c680dd1d4ab.mov

RainEggplant commented 1 year ago

Hi @Vadko! Thank you for your pull request.

I really appreciate your contribution and have made some modifications to the code to fix a bug in the asynchronous logic. Here are the changes:

Changes: 1. I used a global counter `_queueLength` to track the number of queued requests. Because of asynchronous execution, a new request may arrive before previous ones are added to the queue, causing `Queue.getQueueLength` to undercount the actual number of queued requests. 2. I removed sending the queue position in the first receipt message because the position cannot be determined before the request is added to the queue. 3. I removed `_orderRequestsQueue` because it seems unnecessary, given that there can only be one `_updateQueue` function running at a time due to the `_apiRequestsQueue`. Update: fixed another problem in (4d1479f9dc1f9e4410f80d92071b4c118d537b97)

Once again, thank you for your contribution, and please let me know if you have any questions or feedback.

Vadko commented 1 year ago

Hey! Changes seems legit to me, thank you.

Reason for using queue implementation for updating order is to avoid sequence execution of updating order and instead update it in parallel batches with quantity of 20. Currently, when there is no lot of requests it could work stable, but for example if quantity of requests will increase to 50-70 for example this can reduce performance.

RainEggplant commented 1 year ago

Oh, I see. I'll add it back.

But I think we shouldn't use await here, as that will make the execution sequential. What do you think?

https://github.com/Vadko/chatgpt-telegram-bot/blob/b4e0f8d4ea51374c5dd37586e732c761a701d885/src/handlers/chat.ts#L151

Vadko commented 1 year ago

I think await here doesn't wait for execution of promises execution but instead just for adding promise to queue. But, anyway, await isnt required here.

RainEggplant commented 1 year ago

Hi, after researching the TS doc, I found Queue.add just forwards the promise that the input function returns, not returns a promise to join the queue, which I tested and found to be true. So I'll drop the await here.