On Fri, 4 Jan 2008 23:02:06 +0100 "Cedric BAIL" <[EMAIL PROTECTED]> babbled:
> Hi, > > I am looking for a way to improve edje file load time. Right now a > large amount of time is logically lost in _eet_data_descriptor_decode. > So looking at its profile, most of the time is lost in manipulating > string. understandable - this is the parser that takes a nested and tagged set of data chunks and turns it into in-memory data structure goodness. > It took me sometime to understand why we need to strdup all we shouldn't be strdup()ing - we should be evas_stringshare_add()ing. that's what the code does. the string allocator is set to evas_stringshare_add(). this does a hash lookup (intended for read-only string tokens) and reference count of string objects. it will only allocate new ones if not already there. > EET_T_STRING before returning them. In fact, it's only needed when the > Eet_File_Node is compressed, in other case it's not needed. We could sure - though .edj files will get markedly bigger without compression. what you win on strdup() (or memcpy) you lose on increased disk IO - remember disks are on the order of 1/20th or 1/50th or 1/100th or even slower than memory. sure - once in cache you don't suffer this, but now we consume more disk cache as such. so we need to be careful here. > just return a pointer to the string and it will live for as long as > someone reference the eet file. So inside eet itself its not needed to > malloc/free all EET_T_STRING (as the data is valid during all the > runtime of _eet_data_descriptor_decode). sure. though note - this means we need to keep an open file handle to the file - that means if you load 2000 .edj files (eg a directory with 2000+ .edj icons) we will need to consume 2000+ file descriptors. file descriptor counts have limits - often much less than the amount of ram you have, so we need to also be careful here. > Outside of Eet, we have two possibilities : > - we don't change anything (just duplicating all EET_T_STRING in > str_alloc like we do now) > - we can pass a flag to let the application know that the pointer > will live as long as the Eet file referenced. AND we can't compress. > The second solution will break all application using > Eet_Data_Descriptor_Class as we will need to change str_alloc > prototype. I know that Edje and E are using Eet directly, perhaps Rage > also. But the first change could be minimal, just a small wrapper that > ignore the flag and always dup, will do the job. > > For Edje, most of EET_T_STRING are in "edje_file" Eet_File_Node and > guess what it's compressed. So it will require modification to > edje_cc_out.c to choose if we want to compress this Eet_File_Node or > not (could depend on a command line parameter for example). this would be all the strings for the collection names in the edj file. an index as to where to find them. > I did a quick implementation of this idea, and load time did improve > (around 10% global speed improvement in my test case). > eet_data_descriptor_decode doesn't use as much time in load process as > before (from 30% of total load time to 15%). The draw back is edje > file size increase, as an example for Enlightenment theme it will > increase from 2.5M to 2.6M. > > As making a clean patch for this will take time, I would like to > have comments and feed back on this idea before. 10% is a speedup - but not too much of one. gustavo has a very good point of being able to use the file directly and let mmap() handle it. though beware that mmap is only as granular as a page so we would want to cram all our data into page-aligned segments. as you say - strings are repeated a lot. that's because they are written 'verbatim". eet's output has no string "dictionary". it writes the actual string (either the element tag name or content). so as such - eet_data.c (the stuff that handles data encoding/decoding) which it layered on top of normal eet stuff, would need to be extended/changed to allow a decoder to request strings as they are now (duplicated via the duplicator callback - whatever it is. for edje and e it's evas_stringshare_add ()), OR to be returned direct from the file with the understanding you will NEED to keep the file handle open. in fact we can chnage this without breaking anything. look at Eet_Data_Descriptor_Class - it's designed to be able to be versioned and extended over time. you can add new "str_alloc" and "str_free" funcs at the end - and increment eet's EET_DATA_DESCRIPTOR_CLASS_VERSION. we can add str_direct_alloc() and str_direct_free() members - they just return pointers to a string dictionary segment that holds ALL the string tokens for a eet data descriptor encoded chunk, so the string itself in the chunk is now just a reference offset into the string table (eg 82 bytes into the string table is the string u want), so all strings are now 4byte (32bit) offsets into this chunk (ok - limit of 4GB of strings per chunk. i think we'll live). if set these funcs to non-NULL AND set str_alloc() and str_free() to NULL - then eet will return direct references form the eet file data - you DO get passed the pointer to the chunk on alloc and free, but you just do nothing but return it again. *IF* you want to can strdup it - but that's your choice. the func callback lets the app make that call. anyway - that is the data descriptor side. the other size is the eet file container format itself. this also has no dictionary for strings for keys. every key is in fact meant to be unique, so there is almost no point having a dictionary table - they can be inlined. the problem here is that for the eet header and directory - we do load and duplicate it. we do not use it as-is. as you said in a later mail- we could move strings to a table of their own. the only reason to move them is perhaps to split data that needs byteswapping (network to native byte order for example) from data that doesn't to minimise copy-on-write page duplication. 1. 32bit align each directory entry. currently they are not aligned. we do read things safely. the name entires inlined are the problem here. 2. load and parse (byteswap) the dir table as you suggest into ram - this will need to be malloced, BUT instead of mallocing 1 file node per entry, we can alloc 1 big array (saving # of allocs and thus gaining speed), then as you suggest - strings for the name member of each file node can be a pointer into the mmap()ed string table. i don't see this as a problem at all. lets improve things it will make older libeets not be able to read it - but that's ok. we just change the magic number for the eet file. the other option is to drop backwards compat for new libeet to read older eet files? i think maybe we should separate the 2 parsers and code paths so we can keep both - for now, and then drop the old one in 3 or 6 or whatever months (to save maintaining it - definitely drop it by 1.0.0). so as such - i would FIRST work on changing the eet container format (the later one here). this will cause new eet files to be written in this format and backwards-compatibility will allow eet to read older files. this is not as big a chunk of work as the data descriptor stuff - not as complex. i then would change the data descriptor stuff to allow direct-returns of string pointers IF the data is not compressed. if it is - it will probably need to fail and return NULL. -- ------------- Codito, ergo sum - "I code, therefore I am" -------------- The Rasterman (Carsten Haitzler) [EMAIL PROTECTED] ------------------------------------------------------------------------- This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2008. http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/ _______________________________________________ enlightenment-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/enlightenment-devel
