IRC logs of #tryton for Saturday, 2012-03-10

chat.freenode.net #tryton log beginning Sat Mar 10 00:00:02 CET 2012
-!- zxq9(~zxq9@FL1-119-244-167-196.okn.mesh.ad.jp) has left #tryton04:41
albertcacedk: you there?12:05
cedkalbertca: yes13:08
albertcaI was taking a look at: http://codereview.tryton.org/19500113:08
albertcaMore precisely: http://codereview.tryton.org/195001/diff/19001/payment_term.py#newcode24713:09
albertcashouldn't that be 8 spaces like I used?13:09
cedkalbertca: 1 open parenthesis = 4 spaces13:10
cedkalbertca: you can check with http://www.b2ck.com/~ced/python.vim13:13
albertcaok.13:14
albertcaanother thing. Regarding http://codereview.tryton.org/266002/13:15
albertcaI've taken a look of what was the reason for such a big performance improvement13:16
albertcait does not make much sense to have an improvement between 25% (2000 new customers) and 40% (19000 new customers) with sqlite13:16
albertcaa good part of the problem is in modelstorage.py create()13:17
albertcain fact the problem is __clean_xxx2many_cache13:17
albertcait takes 1.5 secons out of 7.5 for 2000 create calls13:17
albertcaspecifically it is the loop that creates the to_clean list in there13:18
albertcaI don't know the internals of this yet but I thought you may be interested because that function is also called in other places13:18
cedkalbertca: it is the cost for having a global cache on cursor13:19
albertcaas I don't know the internals I didn't know if there was place for optimizations in there13:21
albertcamaybe disabling that would make sense for database loading, for example, I'll put in my todo list to do some benchmarks13:22
cedkalbertca: I really don't like to do such disable13:22
cedkalbertca: the system must always behave the same way because it is a framework13:23
albertcaat least it would be good to have some benchmarking13:23
cedkalbertca: but of course your patch for create is the right way13:23
albertcaagain, I don't know the internals, but it makes me think a little bit the fact that a loop of creation of partners13:25
albertcamake it loose so much time trying to find if it has to clear the cache (the to_clean list)13:26
albertcaonly to find that it has to clean nothing :(13:26
albertcawill try to learn the architecture, though..13:27
cedkalbertca: that's why instead of a loop it is better to pass a list of values13:27
cedkalbertca: it is just the same in SQL , it is better to make an insert with many values than calling insert for each one13:28
cedkalbertca: wait, perhaps the changeset 22cc25d44a42 makes this clean method useless13:32
albertcacedk: Ah! I knew you'd take a look at it anyway ;-)13:32
albertcacedk: do you mean: "Don't store BrowseRecord in cursor cache"13:33
cedkalbertca: before we stored BrowseRecord in the cursor cache but there was some issue with that13:34
albertcacedk: so you only store values now?13:34
cedkalbertca: the __clean_xxx2many_cache is there to remove xxx2many value from the cache13:34
cedkalbertca: yes in the cursor cache but we store Browse in the local cache of the BrowseRecordList13:35
cedkalbertca: it will be interested to see if it happens that the method delete something13:36
albertcacedk: AFAIU __clean_xxx2many_cache clears the cursor cache, not Browse one, isn't it13:36
cedkalbertca: yes13:36
albertcacedk: I haven't checked but in my samples I doubt it does because it's a simple create loop13:37
cedkalbertca: the browse cache must be managed by the developer13:37
cedkalbertca: yes of course, I mean testing in real case usage (create sale, invoice etc)13:37
albertcacedk: do you mean that __clean_xxx2many_cache would be useless now with all operations? (create, write, delete)?13:37
cedkalbertca: or even database creation13:37
cedkalbertca: yes13:38
albertcacedk: That's easy to check. I can add an exception there and create a new database, and use it for some days13:38
cedkalbertca: I start tryton test with a print statement13:39
cedkalbertca: it is called for empty list but it is an error in BrowseRecord that store [] instead of empty BrowseRecordList13:42
cedkalbertca: seems to be fixed13:43
albertcacedk: what does that mean? :)13:43
cedklet's see if scenario still works13:43
cedkalbertca: if I don't store empty list in cursor cache for xxx2many fields, the clean method never find someting to clean13:44
cedkit is always good to have other eyes looking at the code13:45
cedkalbertca: by the way, you see it is better and faster to talk on irc :-)13:48
albertcacedk: sure13:52
albertcacedk: just I don't usually remember :)13:53
cedkalbertca: autoconnect :-)13:53
albertcacedk: then, I don't remember it's in there13:57
albertcacedk: believe me, I've tried to get used to it for years13:57
cedkalbertca: so today is a big day13:58
albertcacedk: yeah13:59
albertcacedk: btw, I've realized that it is now possible to upload patches for multiple modules in a single codereview. How do you do that?14:04
cedkalbertca: nicoe write a patch for hgreview14:04
albertcacedk: thanks. Will update my hgreview version then14:05
cedkalbertca: but the patch is not completly ready14:05
albertcaok14:05
cedkalbertca: it is not yet in the repo14:05
cedkalbertca: http://codereview.tryton.org/241001/14:05
cedkACTION scenario testing started14:07
albertcarunning test_account_Invoice.py performance results:14:09
albertcatwo runs with tip: 19.921s & 19.460s14:10
albertcatwo runs with empty __clean_xxx2many_cache: 17.059s & 16.943s14:10
cedkalbertca: of course database creation involve a lot of create calls14:12
albertcatip: 19.6905, empty clean: 17.001 => 13.6 % improvement14:12
albertcacedk: hope we can remove those!14:12
cedkI think we can14:13
albertcaMore performance results. Inserting 2000 parties:14:20
albertcatip: 7.74 seconds14:20
albertcanew list create patch: 5.82514:21
albertcatip + empty clean: 6.4814:21
albertcatnew list create + empty clean: 5.3614:21
albertcanew list create patch: 25% improvement14:23
albertcatip + empty clean: 16%14:23
albertcalist create + empty clean: 30%14:23
meanmiciocedk:ping21:41
meanmicioI think we should have the fields in the tree view expanded by default21:42
cedkmeanmicio: I don't understand21:43
meanmiciocedk : when looking at any tree view, the field names are shortened. This might create issues, and is not easy to read21:44
meanmiciocedk : So we have to expand them manually.21:45
cedkmeanmicio: I don't understand what you want21:46
meanmiciobasically, I would like to see all the fields labels in the tree view without having to expand them manually21:47
meanmiciocedk : So it's easier to read and avoid confusion.21:47
cedkmeanmicio: I don't know how it is possible21:48
cedkmeanmicio: but the system remember the column size per user21:54
meanmiciocedk: you have "fill" to expand the very last column. Maybe we can do something for all21:58
meanmiciocedk : so, something general for the tree tag.21:59
cedkmeanmicio: I don't understand what is expand for non last column22:00
meanmiciocedk: basically, computing the witdth for each field depending on their number of characters22:04
cedkmeanmicio: that will cost too much22:05
meanmiciocedk : not based on the field values, but on the label size22:07
cedkmeanmicio: not easy but doable22:10
meanmiciosomething like the method set_expand(expand) looks like it would do it ..22:13
cedkmeanmicio: the difficulty is that it takes pixels22:15
meanmiciocedk: but set_expand looks like it will take all the available extra space, distributed among all the fields that use it22:16
meanmiciocedk : so if that option is set at any specific column it would expand it as much as possible. Or so it looks.22:17
cedkmeanmicio: need to be tested, in the code there is a comment that it doesn't work well when resize columns22:19
meanmiciocedk : ok. should we try it ? Probably we can use it in one field, resize it and check if it works ok or not22:21
cedkmeanmicio: why not22:23
meanmiciocedk : great !22:23

Generated by irclog2html.py 2.11.0 by Marius Gedminas - find it at mg.pov.lt!