meltwater / served

A C++11 RESTful web server library
MIT License
710 stars 173 forks source link

Replace RE2 with std::regex #34

Closed jpihl closed 6 years ago

jpihl commented 6 years ago

Hi,

I posted issue #33, but since I didn't get any response I just went with the simplest approach and removed RE2. I hope this is something you have interest in.

Not having RE2 greatly simplifies porting this library to multiple platforms.

The following work has been done in this PR:

I hope you find this work useful.

benjamg commented 6 years ago

@Jeffail is there any reason not to merge this, we could have builds to support both but if it's just a minor performance issue that feels excessive. What was our reasoning in using re2 over std::regex in the first place?

jpihl commented 6 years ago

Exactly my thinking. I'm not very familiar with your code base yet - but my guess is that the strings checked by the regex are fairly short - so I doubt the potential performance hit from switching to std::regex will be noticeable.

Jeffail commented 6 years ago

Hey @jpihl, really sorry, this one slipped under my radar. Originally this was simply a performance pre-optimization thing. In practice performance has never been an issue whereas the build complexity has, so I'm happy to merge. Thanks for taking the time to put this together.

jpihl commented 6 years ago

No problem, my pleasure. Thanks for making such a nice library, and making it so enjoyable to contribute :+1: