ryapric / loggit

Modern Logging for the R Ecosystem
Other
37 stars 2 forks source link

Used fixed matching instead of regex #27

Closed MEO265 closed 6 months ago

MEO265 commented 11 months ago

Hello, I don't know which branch would be the right one for the PR, the develop seems suitable to me.

Closes #26. I saw that you already fixed it (in develop), but this way there is no need to escape the :.

# After reversing escaping ":"
> devtools::load_all()
> microbenchmark::microbenchmark(read_logs(), times = 10)
Unit: milliseconds
        expr     min       lq     mean  median       uq      max neval
 read_logs() 949.762 952.6262 974.7672 960.654 965.1672 1060.263    10

# After changing the sanatizer
> devtools::load_all()
> microbenchmark::microbenchmark(read_logs(), times = 10)
Unit: milliseconds
        expr      min       lq     mean   median       uq      max neval
 read_logs() 524.0726 527.1863 536.9793 528.3646 530.8387 596.9289    10

# Final version
> devtools::load_all()
> microbenchmark::microbenchmark(read_logs(), times = 10)
Unit: milliseconds
        expr      min       lq     mean   median       uq      max neval
 read_logs() 487.6132 492.4244 511.2865 496.9468 499.2765 584.3085    10

The times refer to loading 10,000 log entries

MEO265 commented 11 months ago

I took a closer look at the repo, maybe a PR in master would have been the right thing. What do you prefer @ryapric