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

is the tokenmap can not release #679

Open
baneyue opened this issue Aug 27, 2024 · 19 comments · May be fixed by #682
Open

is the tokenmap can not release #679

baneyue opened this issue Aug 27, 2024 · 19 comments · May be fixed by #682

Comments

@baneyue
Copy link

baneyue commented Aug 27, 2024

What is the reason for the continuous increase in objects in the tokenMap during long-term application operation. Running 12 will take up a lot of memory

企业微信截图_1724720541784

@hannesa2
Copy link
Owner

@hannesa2
Copy link
Owner

This is a minor improvement #680

@hannesa2
Copy link
Owner

hannesa2 commented Aug 27, 2024

But the main question is still there: Why they handle token by tokenNumber and access it by tokenNumber instead of using token itself instead and use a mutableList ?

@hannesa2 hannesa2 linked a pull request Aug 27, 2024 that will close this issue
@baneyue
Copy link
Author

baneyue commented Aug 28, 2024

can you merged into branch v3.x ?

and is it resolve the memory problem? i can see u are only change SparseArray to MutableList.

@baneyue
Copy link
Author

baneyue commented Aug 28, 2024

eclipse/paho.mqtt.android#455

i was use qos 0

@hannesa2
Copy link
Owner

hannesa2 commented Aug 28, 2024

@baneyue
Please test it with
implementation 'com.github.hannesa2:paho.mqtt.android:TokenListByMuteable-3.x-SNAPSHOT'

When your test is successful, I will merge it into 3.x branch

@hannesa2
Copy link
Owner

What is the reason for the continuous increase in objects in the tokenMap during long-term application operation. Running 12 will take up a lot of memory

企业微信截图_1724720541784

Btw, how to see the content of this table on my local running instance ?

@baneyue
Copy link
Author

baneyue commented Aug 28, 2024

I found this problem. When I use the master branch, I won't have this issue, but when I use 3. x, I have this problem.

This should be caused by broadcastReceiver, because my usage scenario is sending a string of data in 1 second.
It is estimated that when the performance is tight on Android 6,
broadcastReceiver is easily lost, resulting in tokenMap not receiving callbacks

so i think need not to replace SparseArray to List。

you can merge the flow to 3.x branch to replace the BroadcastReceiver

@hannesa2
Copy link
Owner

I'm somehow lost here

The pull request solves nothing, but I should merge it? -no-

Master comes with another issue, means a lost receiver? But 3.x us fine?

It's another blueprint that I only should merge existing pull requests and anything else is a waste of time. Right?

@baneyue
Copy link
Author

baneyue commented Aug 28, 2024

Wait, I'm still testing

@baneyue
Copy link
Author

baneyue commented Aug 29, 2024

@hannesa2 this problem may be the qos =0 and when send failure,it will not callback deliveryComplete

@hannesa2
Copy link
Owner

Nice.
To have a clear understanding:

The causing issue is a resulting non-remove of these in popSendDetails ? Am I right ?

        val topic = mapTopics.remove(messageToken)
        val activityToken = mapActivityTokens.remove(messageToken)
        val invocationContext = mapInvocationContexts.remove(messageToken) 

@baneyue
Copy link
Author

baneyue commented Aug 29, 2024

yes, there have two leak place :

`
//MqttConnection.kt

private val mapTopics: MutableMap<IMqttDeliveryToken?, String> = HashMap()
private val mapSentMessages: MutableMap<IMqttDeliveryToken?, MqttMessage> = HashMap()
private val mapActivityTokens: MutableMap<IMqttDeliveryToken?, String> = HashMap()
private val mapInvocationContexts: MutableMap<IMqttDeliveryToken?, String> = HashMap()

`

`
// MqttAndroidClient.kt

private val tokenMap = SparseArray<IMqttToken?>()

`

@baneyue
Copy link
Author

baneyue commented Aug 29, 2024

otherwise

  1. I think it's better to use HashMap or SparseArray instead of MutableList
  2. also, I suggest that all of project use Dispatchers IO replaces Dispatchers Main

@hannesa2
Copy link
Owner

This all comes with uncertain actions, so it's worth to have a test for it in advance and surveil the memory usage in this test. Any ideas how to do so ?

@baneyue
Copy link
Author

baneyue commented Aug 29, 2024

u can publish a message set qos =0, and u device is low network style, run your app long time.

My current plan is to set qos=0 without caching, so there will be no callbacks; A better approach would be to specify a timeout period, after which the token will be removed if there is no callback.

@hannesa2
Copy link
Owner

hannesa2 commented Aug 29, 2024

As far as I understand, you run into this with a lot of publishes qos =0,, which all run into (network?) errors. Am I right ?
The next thing is, how do you measure the memory usage. #679 (comment)

@baneyue
Copy link
Author

baneyue commented Aug 29, 2024

The network is weak or congested, and there has been no disconnection

Actually, I'm not sure what the root cause is, but after running for a long time, some callbacks cannot be received, causing those caches to be unable to be released, and the memory growth is not fast. Through profile analysis, the object continues to grow, which is quite observable for our low-end devices, causing the application to slow down

@hannesa2
Copy link
Owner

It's not a miracle, when you do a lot on low end device, that it will become slow.
Solutions have different faces, but to know, if a solution is useful, you need an evidence.
I still wait for #679 (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 a pull request may close this issue.

2 participants