| chat.freenode.net #tryton log beginning Sat Mar 10 00:00:02 CET 2012 | ||
| 2012-03-10 04:41 -!- zxq9(~zxq9@FL1-119-244-167-196.okn.mesh.ad.jp) has left #tryton | ||
| 2012-03-10 12:05 <albertca> cedk: you there? | ||
| 2012-03-10 13:08 <cedk> albertca: yes | ||
| 2012-03-10 13:08 <albertca> I was taking a look at: http://codereview.tryton.org/195001 | ||
| 2012-03-10 13:09 <albertca> More precisely: http://codereview.tryton.org/195001/diff/19001/payment_term.py#newcode247 | ||
| 2012-03-10 13:09 <albertca> shouldn't that be 8 spaces like I used? | ||
| 2012-03-10 13:10 <cedk> albertca: 1 open parenthesis = 4 spaces | ||
| 2012-03-10 13:13 <cedk> albertca: you can check with http://www.b2ck.com/~ced/python.vim | ||
| 2012-03-10 13:14 <albertca> ok. | ||
| 2012-03-10 13:15 <albertca> another thing. Regarding http://codereview.tryton.org/266002/ | ||
| 2012-03-10 13:16 <albertca> I've taken a look of what was the reason for such a big performance improvement | ||
| 2012-03-10 13:16 <albertca> it does not make much sense to have an improvement between 25% (2000 new customers) and 40% (19000 new customers) with sqlite | ||
| 2012-03-10 13:17 <albertca> a good part of the problem is in modelstorage.py create() | ||
| 2012-03-10 13:17 <albertca> in fact the problem is __clean_xxx2many_cache | ||
| 2012-03-10 13:17 <albertca> it takes 1.5 secons out of 7.5 for 2000 create calls | ||
| 2012-03-10 13:18 <albertca> specifically it is the loop that creates the to_clean list in there | ||
| 2012-03-10 13:18 <albertca> I don't know the internals of this yet but I thought you may be interested because that function is also called in other places | ||
| 2012-03-10 13:19 <cedk> albertca: it is the cost for having a global cache on cursor | ||
| 2012-03-10 13:21 <albertca> as I don't know the internals I didn't know if there was place for optimizations in there | ||
| 2012-03-10 13:22 <albertca> maybe disabling that would make sense for database loading, for example, I'll put in my todo list to do some benchmarks | ||
| 2012-03-10 13:22 <cedk> albertca: I really don't like to do such disable | ||
| 2012-03-10 13:23 <cedk> albertca: the system must always behave the same way because it is a framework | ||
| 2012-03-10 13:23 <albertca> at least it would be good to have some benchmarking | ||
| 2012-03-10 13:23 <cedk> albertca: but of course your patch for create is the right way | ||
| 2012-03-10 13:25 <albertca> again, I don't know the internals, but it makes me think a little bit the fact that a loop of creation of partners | ||
| 2012-03-10 13:26 <albertca> make it loose so much time trying to find if it has to clear the cache (the to_clean list) | ||
| 2012-03-10 13:26 <albertca> only to find that it has to clean nothing :( | ||
| 2012-03-10 13:27 <albertca> will try to learn the architecture, though.. | ||
| 2012-03-10 13:27 <cedk> albertca: that's why instead of a loop it is better to pass a list of values | ||
| 2012-03-10 13:28 <cedk> albertca: it is just the same in SQL , it is better to make an insert with many values than calling insert for each one | ||
| 2012-03-10 13:32 <cedk> albertca: wait, perhaps the changeset 22cc25d44a42 makes this clean method useless | ||
| 2012-03-10 13:32 <albertca> cedk: Ah! I knew you'd take a look at it anyway ;-) | ||
| 2012-03-10 13:33 <albertca> cedk: do you mean: "Don't store BrowseRecord in cursor cache" | ||
| 2012-03-10 13:34 <cedk> albertca: before we stored BrowseRecord in the cursor cache but there was some issue with that | ||
| 2012-03-10 13:34 <albertca> cedk: so you only store values now? | ||
| 2012-03-10 13:34 <cedk> albertca: the __clean_xxx2many_cache is there to remove xxx2many value from the cache | ||
| 2012-03-10 13:35 <cedk> albertca: yes in the cursor cache but we store Browse in the local cache of the BrowseRecordList | ||
| 2012-03-10 13:36 <cedk> albertca: it will be interested to see if it happens that the method delete something | ||
| 2012-03-10 13:36 <albertca> cedk: AFAIU __clean_xxx2many_cache clears the cursor cache, not Browse one, isn't it | ||
| 2012-03-10 13:36 <cedk> albertca: yes | ||
| 2012-03-10 13:37 <albertca> cedk: I haven't checked but in my samples I doubt it does because it's a simple create loop | ||
| 2012-03-10 13:37 <cedk> albertca: the browse cache must be managed by the developer | ||
| 2012-03-10 13:37 <cedk> albertca: yes of course, I mean testing in real case usage (create sale, invoice etc) | ||
| 2012-03-10 13:37 <albertca> cedk: do you mean that __clean_xxx2many_cache would be useless now with all operations? (create, write, delete)? | ||
| 2012-03-10 13:37 <cedk> albertca: or even database creation | ||
| 2012-03-10 13:38 <cedk> albertca: yes | ||
| 2012-03-10 13:38 <albertca> cedk: That's easy to check. I can add an exception there and create a new database, and use it for some days | ||
| 2012-03-10 13:39 <cedk> albertca: I start tryton test with a print statement | ||
| 2012-03-10 13:42 <cedk> albertca: it is called for empty list but it is an error in BrowseRecord that store [] instead of empty BrowseRecordList | ||
| 2012-03-10 13:43 <cedk> albertca: seems to be fixed | ||
| 2012-03-10 13:43 <albertca> cedk: what does that mean? :) | ||
| 2012-03-10 13:43 <cedk> let's see if scenario still works | ||
| 2012-03-10 13:44 <cedk> albertca: if I don't store empty list in cursor cache for xxx2many fields, the clean method never find someting to clean | ||
| 2012-03-10 13:45 <cedk> it is always good to have other eyes looking at the code | ||
| 2012-03-10 13:48 <cedk> albertca: by the way, you see it is better and faster to talk on irc :-) | ||
| 2012-03-10 13:52 <albertca> cedk: sure | ||
| 2012-03-10 13:53 <albertca> cedk: just I don't usually remember :) | ||
| 2012-03-10 13:53 <cedk> albertca: autoconnect :-) | ||
| 2012-03-10 13:57 <albertca> cedk: then, I don't remember it's in there | ||
| 2012-03-10 13:57 <albertca> cedk: believe me, I've tried to get used to it for years | ||
| 2012-03-10 13:58 <cedk> albertca: so today is a big day | ||
| 2012-03-10 13:59 <albertca> cedk: yeah | ||
| 2012-03-10 14:04 <albertca> cedk: btw, I've realized that it is now possible to upload patches for multiple modules in a single codereview. How do you do that? | ||
| 2012-03-10 14:04 <cedk> albertca: nicoe write a patch for hgreview | ||
| 2012-03-10 14:05 <albertca> cedk: thanks. Will update my hgreview version then | ||
| 2012-03-10 14:05 <cedk> albertca: but the patch is not completly ready | ||
| 2012-03-10 14:05 <albertca> ok | ||
| 2012-03-10 14:05 <cedk> albertca: it is not yet in the repo | ||
| 2012-03-10 14:05 <cedk> albertca: http://codereview.tryton.org/241001/ | ||
| 2012-03-10 14:07 <cedk> ACTION scenario testing started | ||
| 2012-03-10 14:09 <albertca> running test_account_Invoice.py performance results: | ||
| 2012-03-10 14:10 <albertca> two runs with tip: 19.921s & 19.460s | ||
| 2012-03-10 14:10 <albertca> two runs with empty __clean_xxx2many_cache: 17.059s & 16.943s | ||
| 2012-03-10 14:12 <cedk> albertca: of course database creation involve a lot of create calls | ||
| 2012-03-10 14:12 <albertca> tip: 19.6905, empty clean: 17.001 => 13.6 % improvement | ||
| 2012-03-10 14:12 <albertca> cedk: hope we can remove those! | ||
| 2012-03-10 14:13 <cedk> I think we can | ||
| 2012-03-10 14:20 <albertca> More performance results. Inserting 2000 parties: | ||
| 2012-03-10 14:20 <albertca> tip: 7.74 seconds | ||
| 2012-03-10 14:21 <albertca> new list create patch: 5.825 | ||
| 2012-03-10 14:21 <albertca> tip + empty clean: 6.48 | ||
| 2012-03-10 14:21 <albertca> tnew list create + empty clean: 5.36 | ||
| 2012-03-10 14:23 <albertca> new list create patch: 25% improvement | ||
| 2012-03-10 14:23 <albertca> tip + empty clean: 16% | ||
| 2012-03-10 14:23 <albertca> list create + empty clean: 30% | ||
| 2012-03-10 21:41 <meanmicio> cedk:ping | ||
| 2012-03-10 21:42 <meanmicio> I think we should have the fields in the tree view expanded by default | ||
| 2012-03-10 21:43 <cedk> meanmicio: I don't understand | ||
| 2012-03-10 21:44 <meanmicio> cedk : when looking at any tree view, the field names are shortened. This might create issues, and is not easy to read | ||
| 2012-03-10 21:45 <meanmicio> cedk : So we have to expand them manually. | ||
| 2012-03-10 21:46 <cedk> meanmicio: I don't understand what you want | ||
| 2012-03-10 21:47 <meanmicio> basically, I would like to see all the fields labels in the tree view without having to expand them manually | ||
| 2012-03-10 21:47 <meanmicio> cedk : So it's easier to read and avoid confusion. | ||
| 2012-03-10 21:48 <cedk> meanmicio: I don't know how it is possible | ||
| 2012-03-10 21:54 <cedk> meanmicio: but the system remember the column size per user | ||
| 2012-03-10 21:58 <meanmicio> cedk: you have "fill" to expand the very last column. Maybe we can do something for all | ||
| 2012-03-10 21:59 <meanmicio> cedk : so, something general for the tree tag. | ||
| 2012-03-10 22:00 <cedk> meanmicio: I don't understand what is expand for non last column | ||
| 2012-03-10 22:04 <meanmicio> cedk: basically, computing the witdth for each field depending on their number of characters | ||
| 2012-03-10 22:05 <cedk> meanmicio: that will cost too much | ||
| 2012-03-10 22:07 <meanmicio> cedk : not based on the field values, but on the label size | ||
| 2012-03-10 22:10 <cedk> meanmicio: not easy but doable | ||
| 2012-03-10 22:13 <meanmicio> something like the method set_expand(expand) looks like it would do it .. | ||
| 2012-03-10 22:15 <cedk> meanmicio: the difficulty is that it takes pixels | ||
| 2012-03-10 22:16 <meanmicio> cedk: but set_expand looks like it will take all the available extra space, distributed among all the fields that use it | ||
| 2012-03-10 22:17 <meanmicio> cedk : so if that option is set at any specific column it would expand it as much as possible. Or so it looks. | ||
| 2012-03-10 22:19 <cedk> meanmicio: need to be tested, in the code there is a comment that it doesn't work well when resize columns | ||
| 2012-03-10 22:21 <meanmicio> cedk : ok. should we try it ? Probably we can use it in one field, resize it and check if it works ok or not | ||
| 2012-03-10 22:23 <cedk> meanmicio: why not | ||
| 2012-03-10 22:23 <meanmicio> cedk : great ! |
Generated by irclog2html.py 2.17.3 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!