[thread] Integration with Chrono and DateTime (was: [release][1.52] Release Notes)

On Oct 13, 2012 12:10 AM, "Vicente J. Botet Escriba" < vicente.botet@wanadoo.fr> wrote:
Le 12/10/12 21:51, Andrey Semashev a écrit :
On Oct 12, 2012 8:35 PM, "Vicente J. Botet Escriba" < vicente.botet@wanadoo.fr> wrote:
Hi, Why you don't want to depend on Boost.Chrono?
Because I want to minimize dependencies (especially, binary build dependencies).
Me too, I want to limit dependencies :)
Fair enough. Perhaps, we could absract away Boost.Thread from both libraries? Seems like a logical solution for a case like this. For example, Boost.Thread could define an internal interface for integrating with time units from different libraries. Chrono and DateTime would support this interface, and Boost.Thread could also provide the necessary glue for std::chrono integration. I think, Boost.Thread could use its own time units internally and use ADL-found functions to convert from other time units. Boost.Thread will not depend on either timing library then. Does this look viable?
Unlike Boost.DateTime, Boost.Chrono is not needed by my library, so this would really be a false dependency.
If you don't need time related thread facilities you could set BOOST_THREAD_DONT_USE_CHRONO. Would this work for you?
Yes, that's what I'm doing now. I'm using DateTime units for timed functions.
I understand that the dual interface adds more redundancy but does it really require active maintenance? The code has been there for quite some time, it's well tested and doesn't require additional work.
Any addition code has its influence on how the library could evolve. I really would prefer to remove these interfaces for Boost 1.56. I think that 1 year and a half is enough time to move to the new interface.
Then I think library decoupling is the right way even more.

Le 12/10/12 23:28, Andrey Semashev a écrit :
On Oct 13, 2012 12:10 AM, "Vicente J. Botet Escriba" < vicente.botet@wanadoo.fr> wrote:
Le 12/10/12 21:51, Andrey Semashev a écrit :
On Oct 12, 2012 8:35 PM, "Vicente J. Botet Escriba" < vicente.botet@wanadoo.fr> wrote:
Hi, Why you don't want to depend on Boost.Chrono? Because I want to minimize dependencies (especially, binary build dependencies). Note that Boost.Chrono could be configured as a header only library.
Me too, I want to limit dependencies :) Fair enough. Perhaps, we could absract away Boost.Thread from both libraries? Seems like a logical solution for a case like this.
For example, Boost.Thread could define an internal interface for integrating with time units from different libraries. Chrono and DateTime would support this interface, and Boost.Thread could also provide the necessary glue for std::chrono integration. I think, Boost.Thread could use its own time units internally and use ADL-found functions to convert from other time units. Boost.Thread will not depend on either timing library then. Does this look viable? This could be possible but this is not a priority for me, if you want to
BTW, Boost.Thread depends also on Boost.System. Do you avoid already this dependency? provide a patch I will integrate it if the design is simplified, but I don't see how you could provide the time related function with the Boost.Chrono interface in this case.
Unlike Boost.DateTime, Boost.Chrono is not needed by my library, so this would really be a false dependency. If you don't need time related thread facilities you could set BOOST_THREAD_DONT_USE_CHRONO. Would this work for you?
Yes, that's what I'm doing now. I'm using DateTime units for timed functions. If you need time related facilities, then you will need to move to Boost.Chrono before boost 1.56.
I understand that the dual interface adds more redundancy but does it really require active maintenance? The code has been there for quite some time, it's well tested and doesn't require additional work.
Any addition code has its influence on how the library could evolve. I really would prefer to remove these interfaces for Boost 1.56. I think that 1 year and a half is enough time to move to the new interface.
Then I think library decoupling is the right way even more.
You are surely right, but this is not a priority for me. What others think? Best, Vicente

On Oct 13, 2012 2:02 AM, "Vicente J. Botet Escriba" < vicente.botet@wanadoo.fr> wrote:
Note that Boost.Chrono could be configured as a header only library.
AFAIR, it's not the default, so most if not all distributions include it as a binary. Is the header-only version equivalent to the compiled one?
BTW, Boost.Thread depends also on Boost.System. Do you avoid already this dependency?
Me too, I want to limit dependencies :)
Fair enough. Perhaps, we could absract away Boost.Thread from both libraries? Seems like a logical solution for a case like this.
For example, Boost.Thread could define an internal interface for integrating with time units from different libraries. Chrono and DateTime would support this interface, and Boost.Thread could also provide the necessary glue for std::chrono integration. I think, Boost.Thread could use its own time units internally and use ADL-found functions to convert from other time units. Boost.Thread will not depend on either timing library then. Does this look viable?
This could be possible but this is not a priority for me, if you want to
No, Boost.System is also used by Boost.Filesystem and Boost.ASIO, which I also use. And I probably catch and throw Boost.System exceptions somewhere in my code as well. provide a patch I will integrate it if the design is simplified, but I don't see how you could provide the time related function with the Boost.Chrono interface in this case. Ok, I will create a Trac ticket then and see what I can do.

On Oct 13, 2012 2:02 AM, "Vicente J. Botet Escriba" < vicente.botet@wanadoo.fr> wrote:
Note that Boost.Chrono could be configured as a header only library. AFAIR, it's not the default, so most if not all distributions include it as a binary. You are right and now that I the dependency to Boost.System is also
Le 13/10/12 08:22, Andrey Semashev a écrit : header only I'm thinking to change the default.
Is the header-only version equivalent to the compiled one? I will say yes.
BTW, Boost.Thread depends also on Boost.System. Do you avoid already this dependency?
No, Boost.System is also used by Boost.Filesystem and Boost.ASIO, which I also use. And I probably catch and throw Boost.System exceptions somewhere in my code as well. This is also the case for Boost.Thread so the dependency to the binary is unavoidable for now.
Me too, I want to limit dependencies :) Fair enough. Perhaps, we could absract away Boost.Thread from both libraries? Seems like a logical solution for a case like this.
For example, Boost.Thread could define an internal interface for integrating with time units from different libraries. Chrono and DateTime would support this interface, and Boost.Thread could also provide the necessary glue for std::chrono integration. I think, Boost.Thread could use its own time units internally and use ADL-found functions to convert from other time units. Boost.Thread will not depend on either timing library then. Does this look viable? This could be possible but this is not a priority for me, if you want to provide a patch I will integrate it if the design is simplified, but I don't see how you could provide the time related function with the Boost.Chrono interface in this case.
Ok, I will create a Trac ticket then and see what I can do. Thanks, Vicente

On Oct 12, 2012, at 6:01 PM, "Vicente J. Botet Escriba" <vicente.botet@wanadoo.fr> wrote:
Le 12/10/12 23:28, Andrey Semashev a écrit :
On Oct 13, 2012 12:10 AM, "Vicente J. Botet Escriba" < vicente.botet@wanadoo.fr> wrote:
Le 12/10/12 21:51, Andrey Semashev a écrit :
On Oct 12, 2012 8:35 PM, "Vicente J. Botet Escriba" < vicente.botet@wanadoo.fr> wrote:
Hi, Why you don't want to depend on Boost.Chrono? Because I want to minimize dependencies (especially, binary build dependencies).
BTW, Boost.Thread depends also on Boost.System. Do you avoid already this dependency?
Me too, I want to limit dependencies :) Fair enough. Perhaps, we could absract away Boost.Thread from both libraries? Seems like a logical solution for a case like this.
This seems like a nice idea, but isn't the reason for the switch to Chrono C++11 compatibility? ___ Rob

On Oct 13, 2012 1:19 PM, "Rob Stewart" <robertstewart@comcast.net> wrote:
Fair enough. Perhaps, we could absract away Boost.Thread from both libraries? Seems like a logical solution for a case like this.
This seems like a nice idea, but isn't the reason for the switch to
Chrono C++11 compatibility? This is probably true. However, AFAIK Boost.Thread does not work with std::chrono.

Le 13/10/12 13:52, Andrey Semashev a écrit :
On Oct 13, 2012 1:19 PM, "Rob Stewart" <robertstewart@comcast.net> wrote:
Fair enough. Perhaps, we could absract away Boost.Thread from both libraries? Seems like a logical solution for a case like this. This seems like a nice idea, but isn't the reason for the switch to Chrono C++11 compatibility?
This is probably true. However, AFAIK Boost.Thread does not work with std::chrono.
You are right, and I don't have a solution now. Maybe C++11 template aliases could help in this concern. Best, Vicente

On Oct 13, 2012, at 7:52 AM, Andrey Semashev <andrey.semashev@gmail.com> wrote:
On Oct 13, 2012 1:19 PM, "Rob Stewart" <robertstewart@comcast.net> wrote:
Fair enough. Perhaps, we could absract away Boost.Thread from both libraries? Seems like a logical solution for a case like this.
This seems like a nice idea, but isn't the reason for the switch to Chrono C++11 compatibility?
This is probably true. However, AFAIK Boost.Thread does not work with std::chrono.
True, but Boost.Chrono emulates std::chrono AFAIK. ___ Rob

on Fri Oct 12 2012, "Vicente J. Botet Escriba" <vicente.botet-AT-wanadoo.fr> wrote:
Any addition code has its influence on how the library could evolve. I really would prefer to remove these interfaces for Boost 1.56. I think that 1 year and a half is enough time to move to the new interface.
Then I think library decoupling is the right way even more.
You are surely right, but this is not a priority for me. What others think?
In general I'm all for decoupling, but in the case of Chrono I wonder if it's worth the trouble, since it is, after all, standard in C++11. Also, I bet C++03 implementations are going to start shipping with approximations for all the C++11 libraries, if they haven't already. -- Dave Abrahams BoostPro Computing Software Development Training http://www.boostpro.com Clang/LLVM/EDG Compilers C++ Boost

On Sat, Oct 13, 2012 at 10:21 PM, Dave Abrahams <dave@boostpro.com> wrote:
on Fri Oct 12 2012, "Vicente J. Botet Escriba" <vicente.botet-AT-wanadoo.fr> wrote:
Any addition code has its influence on how the library could evolve. I really would prefer to remove these interfaces for Boost 1.56. I think that 1 year and a half is enough time to move to the new interface.
Then I think library decoupling is the right way even more.
You are surely right, but this is not a priority for me. What others think?
In general I'm all for decoupling, but in the case of Chrono I wonder if it's worth the trouble, since it is, after all, standard in C++11. Also, I bet C++03 implementations are going to start shipping with approximations for all the C++11 libraries, if they haven't already.
Decoupling has additional potential of reducing compile times. Currently, including boost/thread/mutex.hpp pulls in a great deal of DateTime and Chrono, even if you don't use any timed functions (which is a typical case).

Le 16/10/12 08:35, Andrey Semashev a écrit :
On Sat, Oct 13, 2012 at 10:21 PM, Dave Abrahams <dave@boostpro.com> wrote:
on Fri Oct 12 2012, "Vicente J. Botet Escriba" <vicente.botet-AT-wanadoo.fr> wrote:
Any addition code has its influence on how the library could evolve. I really would prefer to remove these interfaces for Boost 1.56. I think that 1 year and a half is enough time to move to the new interface.
Then I think library decoupling is the right way even more.
You are surely right, but this is not a priority for me. What others think? In general I'm all for decoupling, but in the case of Chrono I wonder if it's worth the trouble, since it is, after all, standard in C++11. Also, I bet C++03 implementations are going to start shipping with approximations for all the C++11 libraries, if they haven't already. Decoupling has additional potential of reducing compile times. Currently, including boost/thread/mutex.hpp pulls in a great deal of DateTime and Chrono, even if you don't use any timed functions (which is a typical case).
The single way I see to decoupling is: * to define a base class B that is independent of of Chrono and DateTime and provides a more restrictive time based interface using e.g. nanoseconds, and * define two classes C and D on top of the preceding class based on Chrono and DateTime The main problem is that the interface of the base class will need to be modified for OS that support better precession than nanosecond. This is what Chrono tried to avoid. Any way if the base_ class is non-public this is a minor issue. In fact the current implementation works a little bit like that? The current class boost::XXX could be the C class as it follows the standard. The base class could be called boost::base_XXX and the DateTime bases interface could be moved to a specific DATE_TIME namespace boost::DATE_TIME::XXX or named boost::DATE_TIME_XXX. Is this close to what you are looking for/proposing? Would you agree to be forced to change the name of the class that still provide the DateTime based interface and the file that includes it? Is it worth doing all these changes just to avoid the dependency on Boost.Chrono? Best, Vicente

On Wed, Oct 17, 2012 at 2:24 AM, Vicente J. Botet Escriba <vicente.botet@wanadoo.fr> wrote:
The single way I see to decoupling is:
* to define a base class B that is independent of of Chrono and DateTime and provides a more restrictive time based interface using e.g. nanoseconds, and * define two classes C and D on top of the preceding class based on Chrono and DateTime
That's pretty much what I'd suggested. Only I was thinking of free functions found via ADL or type traits instead of C and D. Thinking more about it, ADL will probably fail with compilers with two-phase name lookup (gcc), so this leaves us the type traits solution.
The main problem is that the interface of the base class will need to be modified for OS that support better precession than nanosecond. This is what Chrono tried to avoid. Any way if the base_ class is non-public this is a minor issue. In fact the current implementation works a little bit like that?
I think B could be made so that it stores OS-dependent precision internally, so no conversion is required in addition to conversion from Chrono or DateTime. This means that B's precision is not consistent across different platforms, but that is ok if it's not public.
The current class boost::XXX could be the C class as it follows the standard. The base class could be called boost::base_XXX and the DateTime bases interface could be moved to a specific DATE_TIME namespace boost::DATE_TIME::XXX or named boost::DATE_TIME_XXX.
I didn't quite understand that. I was thinking of the following layout: 1. Boost.Thread provides a separate header that contains forward declaration of type traits for plugging in a time library. It contains no includes and may look like this: namespace boost { template< typename TimeT, typename = void > struct thread_time_traits; } The second template parameter is for SFINAE-based specializations resolution, like it is done in Phoenix. 2. Boost.Thread provides another header that defines a thread_time_traits specialization for std::chrono. This header is not included anywhere, users are required to include it themselves if they want to use std::chrono with Boost.Thread. 3. Boost.DateTime and Boost.Chrono define their own specializations of this template. These specializations are included automatically by respective Boost.DateTime and Boost.Chrono headers that define time units. This will make these libraries automatically supported by Boost.Thread. 4. Boost.Thread never includes Boost.DateTime or Boost.Chrono, all time units are accepted as templated types by Boost.Thread methods. thread_time_traits template is used to convert Boost.DateTime and Boost.Chrono units to B (Boost.Thread's internal time representation). This is just a general idea. I realize that Boost.Thread accepts both time points and durations, and thread_time_traits would have to be specialized for both. That's the reason I left the second template parameter to be able to specialize the trait once for all types the time library provides. There are other ways, like first determine the library the time unit belongs to (this can be represented by a tag type) and then specialize the trait on that tag, but that involves more templates and would affect compile performance more.
Is this close to what you are looking for/proposing? Would you agree to be forced to change the name of the class that still provide the DateTime based interface and the file that includes it?
With reservations I made above this looks quite close. I do not propose to change the public interface of DateTime or Chrono. Thread public interface would also remain intact unless users rely on public method signatures (which is a bad idea anyway). No files have to be renamed.
Is it worth doing all these changes just to avoid the dependency on Boost.Chrono?
Yes, I think it is.

Andrey Semashev-2 wrote
On Wed, Oct 17, 2012 at 2:24 AM, Vicente J. Botet Escriba <
vicente.botet@
> wrote:
The single way I see to decoupling is:
* to define a base class B that is independent of of Chrono and DateTime and provides a more restrictive time based interface using e.g. nanoseconds, and * define two classes C and D on top of the preceding class based on Chrono and DateTime
That's pretty much what I'd suggested. Only I was thinking of free functions found via ADL or type traits instead of C and D. Thinking more about it, ADL will probably fail with compilers with two-phase name lookup (gcc), so this leaves us the type traits solution.
yes, I was thinking also to use non-member functions, but this doesn't conforms to the C++11 standard.
The main problem is that the interface of the base class will need to be modified for OS that support better precession than nanosecond. This is what Chrono tried to avoid. Any way if the base_ class is non-public this is a minor issue. In fact the current implementation works a little bit like that?
I think B could be made so that it stores OS-dependent precision internally, so no conversion is required in addition to conversion from Chrono or DateTime. This means that B's precision is not consistent across different platforms, but that is ok if it's not public.
I agree, making it private, avoid a lot of issues.
The current class boost::XXX could be the C class as it follows the standard. The base class could be called boost::base_XXX and the DateTime bases interface could be moved to a specific DATE_TIME namespace boost::DATE_TIME::XXX or named boost::DATE_TIME_XXX.
I didn't quite understand that. I was thinking of the following layout:
1. Boost.Thread provides a separate header that contains forward declaration of type traits for plugging in a time library. It contains no includes and may look like this:
namespace boost { template< typename TimeT, typename = void > struct thread_time_traits; }
The second template parameter is for SFINAE-based specializations resolution, like it is done in Phoenix.
2. Boost.Thread provides another header that defines a thread_time_traits specialization for std::chrono. This header is not included anywhere, users are required to include it themselves if they want to use std::chrono with Boost.Thread. 3. Boost.DateTime and Boost.Chrono define their own specializations of this template. These specializations are included automatically by respective Boost.DateTime and Boost.Chrono headers that define time units. This will make these libraries automatically supported by Boost.Thread.
I don't think this should be desirable, but lets see ...
4. Boost.Thread never includes Boost.DateTime or Boost.Chrono, all time units are accepted as templated types by Boost.Thread methods. thread_time_traits template is used to convert Boost.DateTime and Boost.Chrono units to B (Boost.Thread's internal time representation).
This is just a general idea. I realize that Boost.Thread accepts both time points and durations, and thread_time_traits would have to be specialized for both. That's the reason I left the second template parameter to be able to specialize the trait once for all types the time library provides. There are other ways, like first determine the library the time unit belongs to (this can be represented by a tag type) and then specialize the trait on that tag, but that involves more templates and would affect compile performance more.
Could you show the interface the class B look like with this traits?
Is this close to what you are looking for/proposing? Would you agree to be forced to change the name of the class that still provide the DateTime based interface and the file that includes it?
With reservations I made above this looks quite close. I do not propose to change the public interface of DateTime or Chrono. Thread public interface would also remain intact unless users rely on public method signatures (which is a bad idea anyway). No files have to be renamed.
Is it worth doing all these changes just to avoid the dependency on Boost.Chrono?
Yes, I think it is.
Please, let me know about any design issues you could have while doing this refactoring. Best, Vicente -- View this message in context: http://boost.2283326.n4.nabble.com/thread-Integration-with-Chrono-and-DateTi... Sent from the Boost - Dev mailing list archive at Nabble.com.

On Wednesday 17 October 2012 07:57:33 Vicente Botet wrote:
Could you show the interface the class B look like with this traits?
Ok, I started experimenting with the idea and have put together a sort of proof of concept. Please, find attached the patch that modifies boost::timed_mutex for POSIX systems and also adds the necessary bits to Boost.DateTime. The new headers are attached separately. A few words of summary: 1. The new files are: boost/date_time/thread_support.hpp, boost/thread/detail/thread_time_traits.hpp, boost/thread/posix/time_units.hpp. 2. I had to make a few includes in thread_time_traits.hpp - the time_units.hpp for the current platform, which includes config.hpp and cstdint.hpp. These headers are included by DateTime, Chrono and Thread anyway, so harm is really done. 3. I removed explicit support for boost::xtime. This type is long deprecated so I was wondering if there is need for supporting it at all. If there is, it can be plugged in with another thread_time_traits specialization. 4. A nice outcome of the modification is that timed_mutex interface is now unified with regard to time units, so it is now possible to do: mtx.try_lock_for(boost::posix_time::seconds(2)); which was previously only possible with Chrono units. 5. The thread_time_traits interface appeared quite concise (just one conversion function). I also introduced a tag typedef just in case it is needed for dispatching somewhere, but it's not needed now. I will remove it if no such need arises. Please tell me what you think about it.

Le 20/10/12 16:41, Andrey Semashev a écrit :
Could you show the interface the class B look like with this traits? Ok, I started experimenting with the idea and have put together a sort of
On Wednesday 17 October 2012 07:57:33 Vicente Botet wrote: proof of concept. Please, find attached the patch that modifies boost::timed_mutex for POSIX systems and also adds the necessary bits to Boost.DateTime. The new headers are attached separately. The approach seems interesting. I have however some questions and remarks.
Where the types detail::thread_duration and detail::thread_time are defined? I didn't find it in the attached files. I don't like the idea that we need to change Boost.DateTime neither Boost.Chrono. I will prefer that the Boost.DateTime and Boost.Chrono specific stay in a specific Boost.Thread file. DO you think it is possible to don't change these two libraries? if yes, how? By default the Boost.Chrono interface should be provided including the specific file. Of course it should be possible to disable its inclusion. I don't see why thread_time_traits need the second parameter. Partial template specialization seems enough to me, isn't it?
A few words of summary:
1. The new files are: boost/date_time/thread_support.hpp, boost/thread/detail/thread_time_traits.hpp, boost/thread/posix/time_units.hpp.
2. I had to make a few includes in thread_time_traits.hpp - the time_units.hpp for the current platform, which includes config.hpp and cstdint.hpp. These headers are included by DateTime, Chrono and Thread anyway, so harm is really done.
3. I removed explicit support for boost::xtime. This type is long deprecated so I was wondering if there is need for supporting it at all. If there is, it can be plugged in with another thread_time_traits specialization. If no body is against removing it I'm all for.
4. A nice outcome of the modification is that timed_mutex interface is now unified with regard to time units, so it is now possible to do:
mtx.try_lock_for(boost::posix_time::seconds(2));
which was previously only possible with Chrono units.
This could be seen as an advantage but also as an drawback as we are multiplying the interfaces. I don't want the timed_lock to be possible with Boost.Chrono. How could this be achieved?
5. The thread_time_traits interface appeared quite concise (just one conversion function). I also introduced a tag typedef just in case it is needed for dispatching somewhere, but it's not needed now. I will remove it if no such need arises.
Please tell me what you think about it.
IMO something like this could be introduced. I could say more once I have some of the answers to my previous questions. Thanks for the work done, Vicente

On Saturday 20 October 2012 18:43:26 Vicente J. Botet Escriba wrote:
Le 20/10/12 16:41, Andrey Semashev a écrit :
On Wednesday 17 October 2012 07:57:33 Vicente Botet wrote:
Could you show the interface the class B look like with this traits?
Ok, I started experimenting with the idea and have put together a sort of proof of concept. Please, find attached the patch that modifies boost::timed_mutex for POSIX systems and also adds the necessary bits to Boost.DateTime. The new headers are attached separately.
The approach seems interesting. I have however some questions and remarks.
Where the types detail::thread_duration and detail::thread_time are defined? I didn't find it in the attached files.
It's in time_units.hpp.
I don't like the idea that we need to change Boost.DateTime neither Boost.Chrono. I will prefer that the Boost.DateTime and Boost.Chrono specific stay in a specific Boost.Thread file. DO you think it is possible to don't change these two libraries? if yes, how?
I wanted to keep the code backward compatible with previous Boost releases. Users don't include anything special now to use DateTime or Chrono with Thread, I wanted to keep it that way. And after the change Boost.Thread should not include these headers either, which leaves the only choice of including them in the time libraries. The intergration headers can be moved to Thread but they should be included by DateTime and Chrono and in fact I see them specific to these libraries just as much as specific to Boost.Thread. If you really want it that way, I can do that.
By default the Boost.Chrono interface should be provided including the specific file. Of course it should be possible to disable its inclusion.
All timed functions will be provided, only no time library will be included by Boost.Thread. When you include a particular time library, you automatically enable it in Boost.Thread. That's the idea. You can make the support for Boost.Thread explicit by not including the glue header in the time library, but as I said, this will break backward compatibility. I'm not sure this is a good idea.
I don't see why thread_time_traits need the second parameter. Partial template specialization seems enough to me, isn't it?
You can get away with partial specialization but that will require much more thread_time_traits specializations. For instance, DateTime defined a multitude of different duration types, each deriving from date_time::time_duration, which implements most of it. By using nested _is_boost_date_time_duration type as a tag I specialize thread_time_traits once for all these types.
4. A nice outcome of the modification is that timed_mutex interface is now
unified with regard to time units, so it is now possible to do: mtx.try_lock_for(boost::posix_time::seconds(2));
which was previously only possible with Chrono units.
This could be seen as an advantage but also as an drawback as we are multiplying the interfaces. I don't want the timed_lock to be possible with Boost.Chrono. How could this be achieved?
Why? I actually like timed_lock because it is unified for both relative and absolute timeouts. You can try to detect Chrono somehow in these functions but I don't see the point.
participants (5)
-
Andrey Semashev
-
Dave Abrahams
-
Rob Stewart
-
Vicente Botet
-
Vicente J. Botet Escriba