search

cebe / markdown

A super fast, highly extensible markdown parser for PHP
http://markdown.cebe.cc/
MIT License
998 stars 141 forks source link

Don't use comments? #161

Open spinitron opened 6 years ago

spinitron commented 6 years ago

It's not ideal that this relatively small dependency dictates opcache.save_comments=1 at the web server config level.

I think you could refactor the parseXyz() methods and @marker comments that are organized into classes and traits into \Closures and strings organized into assoc arrays.

What do you think?

cebe commented 6 years ago

does opcache.save_comments=0 really safe some resources or improve speed? In general composition of markdown features could be implemented in a different way, but I do not think it is really necessary.

spinitron commented 6 years ago

Yes, it saves opcache shared memory. The memory load for each PHP file is reduced by the number of bytes of comments in the file, iiuc. I agree that it's not necessary to implement without reflecting on comments but I think it's desirable.

We cannot in general quantify the benefit of setting opcache.save_comments=0 since it depends entirely on the project. But I can offer some general qualitative observations...

spinitron commented 6 years ago

Fwiw, OPcache stats from my servers currently show

Not a huge difference but it's not insignificant.

spinitron commented 6 years ago

Nothing much changed. Detailed opcache status below shows server A and B have almost identical opcache use (no surprise, they have almost identical load). The only important difference is used_memory which is ~10% more with with comments. (This opcache has been up for just over a month.)

stat A B
opcache_enabled true true
cache_full false false
restart_pending false false
restart_in_progress false false
memory_usage
used_memory 39580576 43352104
free_memory 77943056 73007632
wasted_memory 16694096 17857992
current_wasted_percentage 12.43807077407837 13.305240869522095
interned_strings_usage
buffer_size 8388608 8388608
used_memory 3365656 3353072
free_memory 5022952 5035536
number_of_strings 60879 60802
opcache_statistics
num_cached_scripts 1614 1613
num_cached_keys 2225 2226
max_cached_keys 16229 16229
hits 2904881093 3074112339
start_time 1535933633 1535720837
last_restart_time 0 0
oom_restarts 0 0
hash_restarts 0 0
manual_restarts 0 0
misses 99128 160821
blacklist_misses 53734850 60452796
blacklist_miss_ratio 1.8161549426196841 1.928487429157587
opcache_hit_rate 98.18049468407227 98.0663822659208
cebe commented 6 years ago

Thank you for the detailed stats!

Ayesh commented 4 years ago

I'm interested in to see if there is any development in this issue.

Ubuntu, Debian and oenrej/php PHP packages have default configuration opcache.save_comments=1, but I personally turn it off save some opcache space. This is my only blocker that prevents me using this awesome package.

Thank you.

cebe commented 4 years ago

If you want to use this lib without comments, you could extend the markdown flavor class you want to use and override the following method:

https://github.com/cebe/markdown/blob/eeb1bf622e80f337479e00a5db94c56e7cf1326b/Parser.php#L254-L285

if you hardcode the array of markers and function names it should all work with opcache.save_comments=0.

I might add some docs about it and possibly also add a configuration option for it...

cebe commented 4 years ago

this can be solved in PHP 8 by using attributes: https://wiki.php.net/rfc/attributes_v2

spinitron commented 4 years ago

@cebe, yes I'm sure it can. It can be solved also with conventional programming.

But the issue with opcache is actually small enough. Allocate enough memory to opcache and stop worrying about it.

In my case the problem has gotten smaller with time as I use fewer comments. With type hints and good names for functions and parameters, comments can be redundant.