Hi, On 17 January 2017 at 21:32, Gustavo Sverzut Barbieri <barbi...@gmail.com> wrote:
> I didn't check the impl patches, but one thing that worries me is > inner main loops. > > If we're trying to simplify the cases where we add to slstr/freeq and > return a pointer, expecting it to be alive (ie: some API returns > 'const char*' but we need to allocate it, previously we'd keep a > reference elsewhere, like pd->last_str), then it may be released if > some other api runs the main loop. > Yeah. The lifecycle of that string would be even more losely defined. Anything may invalidate it. OTOH if we wait for the main loop to be back to root, it may be taking > too long and memory consumption may go too high. > I'm doing the call to the clear function only once from _ecore_main_loop_iterate_internal if not once_only. This means clear is called from ecore_main_loop_begin() or ecore_main_loop_iterate_may_block(1). Any Eina_Slstr string is meant to be consumed asap. We may explain its lifespan to be "in the current function scope" but with "hidden" inner loops that may not be the case. I don't see a more convenient solution with just C, though. On Tue, Jan 17, 2017 at 3:33 AM, Jean-Philippe André <j...@videolan.org> > wrote: > > Hi, > > > > I just merged those patches. > > If you dislike the idea, please argue it well :) > > > > If you dislike the name "slstr", please come up with nicer ideas, I'm all > > open ! > > > > Btw I believe there is no need for any support in eo or eolian, as the > > strings should be handled just like const char *. > > > > Best regards, > > > > > > On 13 January 2017 at 08:16, Carsten Haitzler <ras...@rasterman.com> > wrote: > > > >> On Thu, 12 Jan 2017 15:29:07 +0200 Daniel Hirt <hirt.da...@gmail.com> > >> said: > >> > >> > Hi, > >> > > >> > On Thu, Jan 12, 2017 at 10:27 AM, Jean-Philippe André < > j...@videolan.org > >> > > >> > wrote: > >> > > >> > > Hello, > >> > > > >> > > > >> > > As mentioned in an earlier email I would like to introduce an > >> auto-deleted > >> > > temporary strings API. For now it is called eina_slstr for > short-live > >> > > strings[1]. The strings will be collected by the main loop after > each > >> > > iteration, or whenever the thread exits, or on a manual call to the > >> clear > >> > > function (eg. from a looping thread). > >> > > > >> > > Firstly, I can not reuse eina_tmpstr for this as it is used from > >> multiple > >> > > threads. I tried. It made make check (eio_monitor) fail. So this > >> requires a > >> > > whole new set of APIs. > >> > > > >> > > > >> > > // The goal is to be able to do something like: > >> > > ERR("This object %s is going crazy", object_string_get(obj)); > >> > > > >> > > // With something like: > >> > > const char *object_string_get(Eo *obj) > >> > > { > >> > > return eina_slstr_printf("%s@%p (%s)", > >> > > efl_class_name_get(obj), obj, efl_name_get(obj)); > >> > > } > >> > > > >> > > And not care about freeing the string. In an EO file we would just > >> return a > >> > > "string" and the caller would need to then strdup() it to keep it > >> alive. > >> > > > >> > > A del() function is optional (there is none in my patch). > >> > > > >> > > > >> > > A working implementation is in my devs/jpeg/work branch. > >> > > > >> > > > >> > > For this, I could reuse the Eina_FreeQ introduced by raster but add > a > >> new > >> > > behaviour. For now it's purely used for debugging as a "safe" call > to > >> free > >> > > (although it is arguable whether it will not in fact make debugging > >> harder > >> > > in some cases). > >> > > > >> > > I'd like to introduce a POSTPONED mode where the freeq is not > >> equivalent to > >> > > free() but rather queues up an item that will be collected at the > next > >> call > >> > > to the clear() function. Ecore main loop would clear automatically > for > >> the > >> > > main thread. Other threads would clear on exit automatically. > >> > > > >> > > This kind of queue would need to be thread local. Failure to clear > the > >> > > queue from a looping thread would lead to massive memory leaks. > >> > > > >> > > > >> > > (optional) Another idea would be to have a MAIN_LOOP mode where the > >> item to > >> > > be deleted will be pushed to a thread-safe queue flushed from the > main > >> > > loop. (note: this is just an idea, my patches don't include this). > >> > > > >> > > > >> > > Alternatively eina_slstr can be implemented without a freeq, and > keep > >> the > >> > > freeq exclusively for a "safe" replacement to free(). > >> > > > >> > > > >> > > Thoughts? > >> > > > >> > > > >> > I'd prefer to not piggyback on freeq. Won't a single loop iteration > take > >> A > >> > LOT longer > >> > to free the temporary string than what you would normally have if > you'd > >> just > >> > "strdup + use it + free" it? Just thinking of the amount of temporary > >> > memory piling > >> > up. > >> > Anyway the solution feels a bit artificial, unless I'm not getting the > >> > purpose of > >> > freeq. > >> > >> no no. this i think is where you get confused. there is by default a > single > >> global freeq run by the mainloop. *THIS* freeq is intended to "replace > >> free" > >> by deferring frees until idle time later on (meaning pointers hopefully > are > >> less aggressively recycled which means we get to .. hmmm hide bugs at > >> runtime > >> or at least survive them more, but also we defer any free overhead/costs > >> into > >> idle time but also if you enable memory pattern debug we can find memory > >> issues > >> easily long before they hit us with "malloc internal crashes" with the > mem > >> pattern debug stuff which i found already found me 1 error). > >> > >> what jp is talking about is CREATING a separate freeq that specifically > >> JUST > >> does these temp strings. in fact create one per thread (the first time > it's > >> needed). the mainloop can run a "clean the queue" and this will clean it > >> if it > >> exists... for the mainloop. but any thread can do the same as well when > it > >> is > >> done/idle or the tls cleanup func ca nuke the queue when the thread > exits. > >> it's > >> a special SEPARATE freeq just for these temporary strings. > >> > >> -- > >> ------------- Codito, ergo sum - "I code, therefore I am" -------------- > >> The Rasterman (Carsten Haitzler) ras...@rasterman.com > >> > >> > >> ------------------------------------------------------------ > >> ------------------ > >> Developer Access Program for Intel Xeon Phi Processors > >> Access to Intel Xeon Phi processor-based developer platforms. > >> With one year of Intel Parallel Studio XE. > >> Training and support from Colfax. > >> Order your platform today. http://sdm.link/xeonphi > >> _______________________________________________ > >> enlightenment-devel mailing list > >> enlightenment-devel@lists.sourceforge.net > >> https://lists.sourceforge.net/lists/listinfo/enlightenment-devel > >> > > > > > > > > -- > > Jean-Philippe André > > ------------------------------------------------------------ > ------------------ > > Check out the vibrant tech community on one of the world's most > > engaging tech sites, SlashDot.org! http://sdm.link/slashdot > > _______________________________________________ > > enlightenment-devel mailing list > > enlightenment-devel@lists.sourceforge.net > > https://lists.sourceforge.net/lists/listinfo/enlightenment-devel > > > > -- > Gustavo Sverzut Barbieri > -------------------------------------- > Mobile: +55 (16) 99354-9890 > > ------------------------------------------------------------ > ------------------ > Check out the vibrant tech community on one of the world's most > engaging tech sites, SlashDot.org! http://sdm.link/slashdot > _______________________________________________ > enlightenment-devel mailing list > enlightenment-devel@lists.sourceforge.net > https://lists.sourceforge.net/lists/listinfo/enlightenment-devel > -- Jean-Philippe André ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, SlashDot.org! http://sdm.link/slashdot _______________________________________________ enlightenment-devel mailing list enlightenment-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/enlightenment-devel