Re: [shogun] spdlog
Hi, great stuff so far! see my answers below. > On 22 Jul 2019, at 20:48, Ahmed Essam wrote: > > Regarding using spdlog, here are the assumptions: > 1. There will be only one global SGIO object for logging. > 2. Two streams are used: stdout (for all log levels except for MSG_ERROR) and > stderr, and the user can direct any of them. > 3. spdlog will be exposed. That is the user will use spdlog sinks to redirect > streams. I don't see a reason to wrap around them or add options to > manipulate them further. Exposing spdlog is more flexible. So we can use > "dist_sink" for example to redirect to multiple sinks. > > Some questions: > 1. Are these assumptions reasonable? yes > 2. Should we use fmtlib? I currently format using sprintf, but fmtlib is > supposedly similar in runtime speed, and adds a little overhead in compile > time (header library with compile time format checking!). The downside (for > us) > is that it uses python-like formatting. i would go with fmtlib (see your later comment about c++20 standard having it natively) but of course this will result a lot of changes in the code (due to formatting difference) > 3. Should we get rid of macros? i’d do it :))) and exposing spdlog to everybody is a huge ++ in my opinion as that gives the user a lot of flexibility about where and how things are logged.
Re: [shogun] spdlog
see inline Am Di., 23. Juli 2019 um 14:39 Uhr schrieb Ahmed Essam < theartful...@gmail.com>: > Hi Fernando, > > I think Heiko's point was regarding the global Shogun object you mentioned >> in the first e-mail. > > > Any logging function in SGIO will be thread safe and non-blocking. > Changing the way message is printed and any other options won't be (it can > be easily fixed, but I don't think this is needed). > Great! All good with the singleton then! > > I think source location is actually quite important for logging. There >> should be some alternative keeping it, while allowing us to get rid of >> macro's if that's worth. >> > > I agree. There is "std::expermintal::source_location" but that's not > implemented yet. And there are GCC's extension methods that can do the > same, but that's compiler dependent. If source location is needed, then > it's best to keep the macros. > How do other libs do this? While I intuitively agree that line location is useful for logging, I in practice mostly use gdb if I wanna know what is going on. Logging to me seems more useful for higher level of infos... So long story short. I am OK with dropping them. (I was the one pushing for adding them back 10 years ago with Sören ;) ... but I didnt know how to handle gdb back then :D > > About using fmtlib for "python-like syntax instead of or in addition to >> printf's", what do you think? > > > I like fmtlib, and it's going to be part of C++20, so I don't see why not > :"D. > I think we can go for it in that case :) > > On Tue, Jul 23, 2019 at 2:59 PM Fernando J. Iglesias García < > fernando.iglesi...@gmail.com> wrote: > >> Hey Ahmed, >> >> On Mon, 22 Jul 2019 at 23:33, Ahmed Essam wrote: >> >>> Thanks for the feedback! >>> >>> spdlog is friendly with multi threaded code. In fact, I currently use in >>> the PR (as viktor suggested) an async logger, which does the logging in a >>> different thread altogether. >>> >> >> I think Heiko's point was regarding the global Shogun object you >> mentioned in the first e-mail. >> >> >>> Regarding the exposure of spdlog, I can add the functionality for >>> directing streams to files, and we might discuss what other functionality >>> is expected. However, exposing spdlog would give a huge amount of >>> flexibility, as you can easily do whatever you want, thanks to their >>> amazing interface and built-in sinks. >>> >>> Note that getting rid of macros means getting rid of source location >>> (line, function, and file name). >>> >> >> I think source location is actually quite important for logging. There >> should be some alternative keeping it, while allowing us to get rid of >> macro's if that's worth. >> >> About using fmtlib for "python-like syntax instead of or in addition to >> printf's", what do you think? >> >> >>> >>> >>> On Mon, Jul 22, 2019 at 11:24 PM Heiko Strathmann < >>> heiko.strathm...@gmail.com> wrote: >>> Hi! Great initiative! See below Am Mo., 22. Juli 2019 um 19:49 Uhr schrieb Ahmed Essam < theartful...@gmail.com>: > Regarding using spdlog, here are the assumptions: > 1. There will be only one global SGIO object for logging. > Similar problems as for random: This means that multithreaded code will also use this for logging, and we will need some sort of critical around the log output. I wonder how that would affect performance. Also I wonder how other (multithreaded) libs do this? Some minimal research might be good here. > 2. Two streams are used: stdout (for all log levels except for > MSG_ERROR) and stderr, and the user can direct any of them. > That sounds very reasonable 3. spdlog will be exposed. That is the user will use spdlog sinks to > redirect streams. I don't see a reason to wrap around them or add options > to manipulate them further. Exposing spdlog is more flexible. So we can > use > "dist_sink" for example to redirect to multiple sinks. > I am not too sure about this. Backend libraries tend to change over the years and explicitly adding those into the codebase usually causes headaches later on. So I'd actually prefer some plugin like thing, but I can probably be convinced ;) > > Some questions: > 1. Are these assumptions reasonable? > 2. Should we use fmtlib? I currently format using sprintf, but fmtlib > is supposedly similar in runtime speed, and adds a little overhead in > compile time (header library with compile time format checking!). The > downside (for us) is that it uses python-like formatting. > Seems fine to me > 3. Should we get rid of macros? > I think that would be nice. I guess they were used to switch off things at compile time. We don't really do that anymore. >>>
Re: [shogun] spdlog
Hey Ahmed, On Mon, 22 Jul 2019 at 23:33, Ahmed Essam wrote: > Thanks for the feedback! > > spdlog is friendly with multi threaded code. In fact, I currently use in > the PR (as viktor suggested) an async logger, which does the logging in a > different thread altogether. > I think Heiko's point was regarding the global Shogun object you mentioned in the first e-mail. > Regarding the exposure of spdlog, I can add the functionality for > directing streams to files, and we might discuss what other functionality > is expected. However, exposing spdlog would give a huge amount of > flexibility, as you can easily do whatever you want, thanks to their > amazing interface and built-in sinks. > > Note that getting rid of macros means getting rid of source location > (line, function, and file name). > I think source location is actually quite important for logging. There should be some alternative keeping it, while allowing us to get rid of macro's if that's worth. About using fmtlib for "python-like syntax instead of or in addition to printf's", what do you think? > > > On Mon, Jul 22, 2019 at 11:24 PM Heiko Strathmann < > heiko.strathm...@gmail.com> wrote: > >> Hi! >> >> Great initiative! See below >> >> Am Mo., 22. Juli 2019 um 19:49 Uhr schrieb Ahmed Essam < >> theartful...@gmail.com>: >> >>> Regarding using spdlog, here are the assumptions: >>> 1. There will be only one global SGIO object for logging. >>> >> Similar problems as for random: This means that multithreaded code will >> also use this for logging, and we will need some sort of critical around >> the log output. I wonder how that would affect performance. Also I wonder >> how other (multithreaded) libs do this? Some minimal research might be good >> here. >> >> >>> 2. Two streams are used: stdout (for all log levels except for >>> MSG_ERROR) and stderr, and the user can direct any of them. >>> >> That sounds very reasonable >> >> 3. spdlog will be exposed. That is the user will use spdlog sinks to >>> redirect streams. I don't see a reason to wrap around them or add options >>> to manipulate them further. Exposing spdlog is more flexible. So we can use >>> "dist_sink" for example to redirect to multiple sinks. >>> >> I am not too sure about this. Backend libraries tend to change over the >> years and explicitly adding those into the codebase usually causes >> headaches later on. So I'd actually prefer some plugin like thing, but I >> can probably be convinced ;) >> >> >>> >>> Some questions: >>> 1. Are these assumptions reasonable? >>> 2. Should we use fmtlib? I currently format using sprintf, but fmtlib is >>> supposedly similar in runtime speed, and adds a little overhead in compile >>> time (header library with compile time format checking!). The downside (for >>> us) is that it uses python-like formatting. >>> >> Seems fine to me >> >> >>> 3. Should we get rid of macros? >>> >> I think that would be nice. I guess they were used to switch off things >> at compile time. We don't really do that anymore. >> >> >
Re: [shogun] spdlog
Thanks for the feedback! spdlog is friendly with multi threaded code. In fact, I currently use in the PR (as viktor suggested) an async logger, which does the logging in a different thread altogether. Regarding the exposure of spdlog, I can add the functionality for directing streams to files, and we might discuss what other functionality is expected. However, exposing spdlog would give a huge amount of flexibility, as you can easily do whatever you want, thanks to their amazing interface and built-in sinks. Note that getting rid of macros means getting rid of source location (line, function, and file name). On Mon, Jul 22, 2019 at 11:24 PM Heiko Strathmann < heiko.strathm...@gmail.com> wrote: > Hi! > > Great initiative! See below > > Am Mo., 22. Juli 2019 um 19:49 Uhr schrieb Ahmed Essam < > theartful...@gmail.com>: > >> Regarding using spdlog, here are the assumptions: >> 1. There will be only one global SGIO object for logging. >> > Similar problems as for random: This means that multithreaded code will > also use this for logging, and we will need some sort of critical around > the log output. I wonder how that would affect performance. Also I wonder > how other (multithreaded) libs do this? Some minimal research might be good > here. > > >> 2. Two streams are used: stdout (for all log levels except for MSG_ERROR) >> and stderr, and the user can direct any of them. >> > That sounds very reasonable > > 3. spdlog will be exposed. That is the user will use spdlog sinks to >> redirect streams. I don't see a reason to wrap around them or add options >> to manipulate them further. Exposing spdlog is more flexible. So we can use >> "dist_sink" for example to redirect to multiple sinks. >> > I am not too sure about this. Backend libraries tend to change over the > years and explicitly adding those into the codebase usually causes > headaches later on. So I'd actually prefer some plugin like thing, but I > can probably be convinced ;) > > >> >> Some questions: >> 1. Are these assumptions reasonable? >> 2. Should we use fmtlib? I currently format using sprintf, but fmtlib is >> supposedly similar in runtime speed, and adds a little overhead in compile >> time (header library with compile time format checking!). The downside (for >> us) is that it uses python-like formatting. >> > Seems fine to me > > >> 3. Should we get rid of macros? >> > I think that would be nice. I guess they were used to switch off things at > compile time. We don't really do that anymore. > >
