On Wed, 24 Jul 2019 16:31:27 +0000 Cedric Bail <ced...@ddlm.me> said:
> On Wednesday, July 24, 2019 3:45 AM, Carsten Haitzler <ras...@rasterman.com>
> wrote:
> > I'm looking at the efl.io design and ... I don't like it.
> >
>
> > (k-s - for you if you're listening).
> >
>
> > I'm not saying it isn't clean or neat. It IS expensive. EO calls are costly.
> > We should avoid designing things that rely on making lots of them. let me
> > give a snippet of code:
> >
>
> > ...
> > efl_event_callback_add
> > (obj, EFL_IO_READER_EVENT_CAN_READ_CHANGED, _th_read_change, NULL);
> > ...
> > static void
> > _th_read_change(void *data EINA_UNUSED, const Efl_Event *ev)
> > {
> > Eo *obj = ev->object;
> > char buf[4096];
> > Eina_Rw_Slice rw_slice = EINA_SLICE_ARRAY(buf);
> >
>
> > while (efl_io_reader_can_read_get(obj))
> > {
> > Eina_Error err = efl_io_reader_read(obj, &rw_slice);
> > if (!err)
> > {
> > // XXX: access rw_slice with eina calls
> > ...
> >
>
> > My problem with this is in the _th_read_change() handling. It requires per
> > "unit" read (here up to 4k) we will have to do 3 eo calls. For I/O that is
> > smaller with lots of little messages this means a callback to
> > _th_read_change() which is kind of unavoidable THEN at LEAST 3 eo calls.
> >
>
> > 1. efl_io_reader_can_read_get() returns true -> data to read.
> >
>
> > 2. efl_io_reader_read() reads data
> >
>
> > 3. efl_io_reader_can_read_get() again next loop returns false.
> >
>
> > Reality is this will be super common. For anything that cares about
> > performance, this will suck. At least for reads it definitely will.
> > Layering anything on top with more classes won't help as they all will
> > internally have to do the above anyway just adding to the cost anyway or at
> > a minimum being the same.
> >
>
> > My take is that the IO class needs to pre-read (how much is a policy
> > tweak/option that can be set on the object via the IO class API) and
> > simply present this slice to the "read" callback. then inside the cb no eo
> > calls are needed to get the data. It's already there, ready to go. This
> > would be a fairly major change to the API, but IMHO necessary. Going with
> > what we have right now I think is asking for a "performance disaster" baked
> > right into our API design in this area.
>
> This would also solve another issue we have with this API. As it is, we can
> only have one handler reading. Having the slice provided to the event would
> enable multi handler read. I think we have to fix for both reason this API.
> It doesn't seems to big of a change (famous last word).
Correct and very good point. It also makes the API better for multiple readers
thus allowing multiple parsers for an incoming stream for example with a mixin
or an object listener.
It's better to make these changes early before we have lots of code built on top
when it is more painful. Just wanting to see if there are other views on this
or what should be done instead to see if there are better alternative paths. If
nothing turns up in a reasonable time then I might get to it. I have another
todo item relating to thread shutdown policy to do that I've added to my list.
--
------------- Codito, ergo sum - "I code, therefore I am" --------------
Carsten Haitzler - ras...@rasterman.com
_______________________________________________
enlightenment-devel mailing list
enlightenment-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/enlightenment-devel
