IRC logs of #tryton for Saturday, 2012-03-10 #tryton log beginning Sat Mar 10 00:00:02 CET 2012
2012-03-10 04:41 -!- zxq9( 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:
2012-03-10 13:09 <albertca> More precisely:
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
2012-03-10 13:14 <albertca> ok.
2012-03-10 13:15 <albertca> another thing. Regarding
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 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:
2012-03-10 14:07 <cedk> ACTION scenario testing started
2012-03-10 14:09 <albertca> running 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 2.17.3 by Marius Gedminas - find it at!