| freemangordon | right, ChatWindow needs lots of refactoring/cleanup, | 00:05 |
|---|---|---|
| freemangordon | but, time for zzz, night! | 00:05 |
| dsc_ | gn | 00:08 |
| freemangordon | dsc_: why explicit clear here: https://github.com/maemo-leste/conversations/blob/master/src/chatwindow.cpp#L251 ? | 07:29 |
| freemangordon | ok, do you mind if I refactor ChatWindow to remove m_chatMessage ? | 07:33 |
| freemangordon | anyway it seems broken https://github.com/maemo-leste/conversations/blob/master/src/chatwindow.cpp#L151 | 07:34 |
| freemangordon | dsc_: https://pastebin.com/EqFMMhKG | 09:02 |
| freemangordon | this happens if I do offline->online | 09:02 |
| Wizzup | freemangordon: that could be what I mentioned earlier, using a channel that is invalidated, but I would reckon that we also removing it from the qmap when it is invalidated | 10:46 |
| dsc_ | freemangordon: regarding specific TP questions, I am not a fan of lib/tp.cpp, tpqt, and rtcom in general, so asking me why hasChannel segfaults - i have no idea. Historically Wizzup has been working more on that side of the code anyway. | 11:20 |
| uvos__ | Wizzup: the bouncing vhold on the xt91x is global everywere | 11:21 |
| dsc_ | I can find out why it would segfault, but it takes me like +30min to even get familiar again with what happens, the terminology, etc. | 11:21 |
| uvos__ | Wizzup: just a note btw tdlib has som issues in conversations | 11:22 |
| uvos__ | 1. only remote party notifications are fetched | 11:22 |
| uvos__ | if i have a conversation with smeone via telegram later conversations will only have the messages of the other party, my own messages are missing | 11:23 |
| uvos__ | 2. when im present in leste all messages by sent by anyone are automaticly set to "seen" by leste | 11:24 |
| uvos__ | this is very undesirable as any time the leste device is active all other telegram clients dont get any notification | 11:24 |
| uvos__ | dsc_: conversations sphone module maybe? :P | 11:25 |
| dsc_ | the only reason conversations uses rtcom is so can read a sqlite db, but in Qt we generally use Qt SQL for it | 11:27 |
| dsc_ | well, rtcom goes via dbus, maybe it does some extra things, I am not sure. But I feel like it is unnecessarily complex. | 11:27 |
| dsc_ | (or maybe rtcom does not go via dbus, i dont even know... :P) | 11:28 |
| uvos__ | oh you just mean rtcom-el | 11:31 |
| uvos__ | in pricipal rtcom is the whole stack | 11:31 |
| dsc_ | yes, rtcom-el | 11:31 |
| uvos__ | rtcom-el is pretty wierd yeah | 11:32 |
| uvos__ | its both overgeneered and lacking | 11:32 |
| dsc_ | also we need to mix glib in Qt because of it | 11:32 |
| dsc_ | uvos__: marked as 'seen' is one of the more recent features | 11:35 |
| dsc_ | ill note your description | 11:35 |
| dsc_ | im working on such issues this week | 11:35 |
| Wizzup | uvos__: check @ bouncing | 11:37 |
| Wizzup | uvos__: I agree @ missing your own messages, I think we might ignore those somehow, will look (or dsc will) | 11:37 |
| dsc_ | I was going to check memory increases today but ill wait for fmg's constructor refactor now :P | 11:37 |
| uvos__ | dsc_: ok to be clear the problem is that messages are marked as seen when no conversations window is not open at all | 11:52 |
| uvos__ | merly the present sate is enough | 11:52 |
| uvos__ | tdlib seams to think that all chats are open and active at all times | 11:52 |
| uvos__ | hence no notifications for other devices | 11:53 |
| Wizzup | yeah, this might be what freemangordon fixing in tp-haze | 11:53 |
| Wizzup | and then supporting in conversations | 11:53 |
| Wizzup | (chat state) | 11:53 |
| Wizzup | dsc_: we use rtcom because maemo uses rtcom, like, for everything related to real time communications | 11:54 |
| Wizzup | and yes it does more than just 'log to a database' | 11:54 |
| Wizzup | I think we're past that point now so it would be nice if we could stop having to doubt this frequently :D | 11:54 |
| uvos__ | not a whole lot more really tho | 11:55 |
| uvos__ | for rtcom-el | 11:55 |
| uvos__ | it would be quite feasbale to just implement it again qtsql | 11:55 |
| Wizzup | it's read by notifications too | 11:55 |
| Wizzup | also used for persistent notifications | 11:55 |
| Wizzup | and it does a ton of stuff that you might not know about if you just reimplement it | 11:55 |
| uvos__ | if the interface is getting in the way | 11:56 |
| Wizzup | it is not | 11:56 |
| Wizzup | there is no issue in mixing glib and qt, it's not causing us problems | 11:56 |
| uvos__ | Wizzup: rtcom-el is simply very few lines of code | 11:56 |
| uvos__ | Wizzup: so you could absolutly not miss anything | 11:56 |
| Wizzup | 14k lines of code | 11:58 |
| Wizzup | 8.5k without ui code | 11:59 |
| Wizzup | so no, it's not very little | 11:59 |
| dsc_ | 09:55 < uvos__> it would be quite feasbale to just implement it again qtsql if the interface is getting in the way | 11:59 |
| dsc_ | yep, this | 11:59 |
| Wizzup | now, as far as the telepathy segfault, I can take a look but I need freemangordon's changes to do so | 12:00 |
| Wizzup | I don't mind looking at the tp code at all which is the divisions we've had before so there's no need to change it now | 12:00 |
| dsc_ | < Wizzup> there is no issue in mixing glib and qt <== this is simply not true, by having glib and Qt you have introduced more dependencies to link against at the minimum, and at the maximum you have made querying a sqlite database a lot more complicated | 12:01 |
| Wizzup | I am not going to have this discussion again, sorry, this is super pointless | 12:01 |
| Wizzup | If you can't see why having to maintain two separate code bases for accessing rt com messages is not a nightmare then I don't know what to say | 12:02 |
| Wizzup | I also don't remember a single crash caused by using both glib and qt | 12:04 |
| dsc_ | there are issues by having this setup and I cannot talk about them | 12:04 |
| dsc_ | or they are ignored | 12:04 |
| dsc_ | which is fine, but I keep bringing them so it is clear that part of the code is not what I would have preferred, e.g when fmg asks to do something I can say "i dont know" :) | 12:05 |
| dsc_ | them up* | 12:05 |
| Wizzup | yes, like I said, I am happy to look at the tp stuff | 12:05 |
| Wizzup | just bounce it to me then | 12:05 |
| Wizzup | tp stuff or rtcom stuff | 12:05 |
| Wizzup | 11:21 < uvos__> Wizzup: the bouncing vhold on the xt91x is global everywere | 12:07 |
| Wizzup | ok, it would be nice if we could try to fix it somehow, I can make some time again this week(end)... we have a lot of nlnet stuff to claim for devices and it's almost all like 99% of the way there (at least that is what it feels like) | 12:08 |
| Wizzup | did you need certain data from android on razr? | 12:08 |
| Wizzup | dsc_: btw it looks to me like _removeChannel should remove the channel from the channels QMap | 12:19 |
| Wizzup | because otherwise we keep invalid channels/pointers around | 12:19 |
| dsc_ | if it keeps the keys around, I did so for a purpose | 12:21 |
| dsc_ | most likely to facilitate the auto-join group feature | 12:21 |
| dsc_ | and/or keep track of which channels it should join, but has not joined yet | 12:21 |
| Wizzup | the channels private qmap is only to keep track of telepathychannels that are valid | 12:22 |
| Wizzup | I don't think it stores anything related to autojoin | 12:22 |
| dsc_ | ill check | 12:22 |
| Wizzup | I think those are different, and if we store things for autojoin we should just store the account + channel string | 12:22 |
| Wizzup | ok, I can also check, I just wanted to communicate what is likely the issue | 12:22 |
| dsc_ | hasChannel checks for key and value | 12:27 |
| dsc_ | as the presence of a key does not mean its valid | 12:28 |
| dsc_ | due to the reasons above | 12:29 |
| dsc_ | thats how it was written anyway | 12:29 |
| dsc_ | we could change that if thats better, im not sure | 12:30 |
| Wizzup | I don't think there is a point to keeping invalidated channels around, right? | 12:30 |
| Wizzup | at least to me that feels like keeping around pointers to freed memory | 12:30 |
| Wizzup | but maybe I am missing something | 12:31 |
| dsc_ | yes that shouldnt happen | 12:33 |
| dsc_ | somewhere it should destroy, and also set nullptr | 12:33 |
| Wizzup | so I think _removeChannel is called when we are informed that the channel is no longer usable, and then telepathyqt just removes it | 12:35 |
| Wizzup | so I think it gets destroyed already (I am not sure, I can check a bit later) and our nulling would be removing it from the qmap | 12:35 |
| dsc_ | there's also leaveChannel() | 12:36 |
| Wizzup | I think leavechannel can just ask to leave the channel and then _removeChannel will be called eventually | 12:36 |
| Wizzup | but that is my bet | 12:36 |
| dsc_ | leaveChannel actually removes the key | 12:37 |
| Wizzup | yeah I saw it does currently, but I am not sure if it has to if _removeChannel does it | 12:37 |
| Wizzup | I think they could both do it though, then one just becomes a no-op | 12:37 |
| dsc_ | right yeah | 12:37 |
| dsc_ | uvos__: regarding point 1 "my own messages are missing", could you re-try with a fresh rtcom db? | 12:50 |
| dsc_ | just as a quick test | 12:50 |
| dsc_ | i suspect that will just work | 12:51 |
| uvos__ | dsc_: shure i will | 12:54 |
| uvos__ | can you remind me where the db is stored again | 12:55 |
| dsc_ | uvos__: ~/.rtcom-eventlogger/ | 12:55 |
| dsc_ | el-v1.db | 12:56 |
| dsc_ | < uvos__> 2. when im present in leste all messages by sent by anyone are automaticly set to "seen" by leste <== 90% sure Conversations does not do this, message are only marked as 'read' when chatWindow is opened, once | 12:56 |
| dsc_ | maybe the telegram plugin does? | 12:57 |
| uvos__ | maybe unfotionatly theres a lot of layers involved here | 12:57 |
| uvos__ | any one of them could be at fault | 12:57 |
| Wizzup | the best way to verify if we get them at all is to look at dbus-monitor when we send a message from a different device | 13:00 |
| Wizzup | then we will know if conversations receives it from tp-haze | 13:00 |
| Wizzup | if it does, then we can fix conversations to store it correctly | 13:00 |
| Wizzup | otherwise we need to look at haze, and if haze doesn't even get it, then we need to look at tdlib purple | 13:00 |
| Wizzup | (which could be tested from within pidgin, without any tp stuff, to see if pidgin gets these messages) | 13:01 |
| dsc_ | i may have identified rtcom-el as the cause for some mem increases (or how conversations uses it) | 16:22 |
| dsc_ | needs further investigation before drawing conclusions though, so brb | 16:23 |
| dsc_ | results seem promising | 17:35 |
| freemangordon | dsc_: as a side note: qt itself uses glib behind the scenes | 17:56 |
| freemangordon | so qt vs glib is simply pointless | 17:56 |
| freemangordon | to me it seems the checks in hasChannel are not enough | 17:57 |
| freemangordon | Wizzup: sure, I can provide the code | 17:57 |
| freemangordon | code changes that is | 17:57 |
| freemangordon | but I am almost sure segfault is unrelated, besides that is calls hasChannel() buy the time channels map contains dangling pointers | 17:58 |
| freemangordon | which should not happen | 17:58 |
| freemangordon | *by the time | 17:58 |
| Wizzup | yes, I think we discussed this backlog | 18:09 |
| Wizzup | dsc_: cool @ memory! | 18:12 |
| arno11 | dsc_: great @ memory !!! | 18:18 |
| dsc_ | i just need to check if its my fault before I explain what I found (so I can prepare everyone making fun of me ;')) | 18:22 |
| dsc_ | but yes pretty sure this improves things | 18:23 |
| dsc_ | but im also not satisfied with QML but thats a different story | 18:23 |
| dsc_ | brb | 18:23 |
| dsc_ | old leak re-fixed: https://github.com/maemo-leste/conversations/commit/a2bb39114de7bd0e2d10125af967034aad6ac608 | 19:46 |
| dsc_ | new leak: https://github.com/maemo-leste/conversations/commit/fe402cca7f02ec7f1d385a3f907de3ad5fffac79 | 19:46 |
| dsc_ | it has a noticeable difference | 19:46 |
| dsc_ | however I am still not satisfied with some memory usage, possibly related to QML/ChatWindow (opening and closing doesnt return the 'expected' amount of memory) | 19:47 |
| dsc_ | so ill continue tomorrow | 19:48 |
| freemangordon | dsc_: qml will not free until GC is run | 20:38 |
| freemangordon | so maybe run GC manually before measuring memory usage | 20:38 |
| Wizzup | dsc_: great work! | 21:21 |
| Wizzup | I think we can make a new -devel release with just this | 21:22 |
| freemangordon | Wizzup: do you want my changes or you could give me a hint where to remove the channel (and when) | 21:32 |
| Wizzup | sure, I did mention a fix in the backlog, which is to do channels.remove(...) in _removeChannel | 21:34 |
| Wizzup | but if you push the code to a branch I can take a look too | 21:34 |
| dsc_ | freemangordon: closing a chatWindow destroys the QML widget in its entirety, but memory usage doesnt return close to what it was | 21:34 |
| Wizzup | dsc_: if you do that a few times, does it still increase? | 21:34 |
| dsc_ | Wizzup: it does, but it stops after a while. its not linear | 21:34 |
| Wizzup | the first time might be some cache in qt / loading new so's | 21:34 |
| Wizzup | hm | 21:34 |
| Wizzup | dsc_: shall I make a new -devel release? | 21:35 |
| dsc_ | yes | 21:35 |
| dsc_ | thanks | 21:35 |
| dsc_ | i also did some testing today with javascript's gc() | 21:38 |
| dsc_ | but I dont have anything to report on that front yet | 21:39 |
| freemangordon | Wizzup: ok, will try | 21:39 |
| dsc_ | Wizzup: i'd like to get one more thing in | 21:50 |
| dsc_ | (if you did not update already) | 21:50 |
| Wizzup | ah | 21:50 |
| Wizzup | ok, let me git tag -d the tag | 21:50 |
| freemangordon | also, I am about to push segfault fix | 21:51 |
| freemangordon | if you like it https://pastebin.com/B7yAWq7C | 21:51 |
| Wizzup | looks good to me (assuming it works) | 21:52 |
| freemangordon | about to test | 21:53 |
| freemangordon | seems to not segfault anymore, going to push it | 21:56 |
| dsc_ | pushed | 22:03 |
| freemangordon | Wizzup: pushed | 22:12 |
| freemangordon | along with chat state functionality | 22:13 |
| Wizzup | cool | 22:17 |
| freemangordon | will you make anotehr release? | 22:18 |
| Wizzup | yes,now | 22:19 |
| freemangordon | ok, thanks | 22:19 |
| freemangordon | dsc_: pressing ENTER key does not send messages in either VM or D4 | 22:20 |
| freemangordon | Wizzup: will try to push haze fixes today | 22:20 |
| dsc_ | freemangordon: its in Settings | 22:20 |
| dsc_ | but it also requires a restart of chatWindow, still need to fix | 22:21 |
| freemangordon | how to access those settings? | 22:21 |
| dsc_ | on the overview window -> menu bar -> Settings | 22:21 |
| freemangordon | hmm, trying to open conversations while a chat is opened just raises that chat | 22:23 |
| arno11 | i just tried the new release: it is really faster to receive and open sms chat window on N900 | 22:53 |
| dsc_ | cool | 22:56 |
| dsc_ | https://github.com/Karry/osmscout-sailfish/blob/master/src/MemoryManager.cpp | 23:03 |
| dsc_ | every 10min it checks device memory, and forces Javascript collection and/or malloc_trim(0) | 23:03 |
| dsc_ | might be interesting to explore | 23:04 |
| dsc_ | but my question is, why Qt keeps some memory after destroying the chatwindow | 23:12 |
| dsc_ | need to investigate that | 23:13 |
Generated by irclog2html.py 2.17.0 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!