»
« home   paste   Anonymous | Login | Signup for a new account 07-28-2017 16:47 CEST
 
* X »
«
GeSHi - Generic Syntax Highlighter Syntax Coloriser for PHP
  

Viewing Issue Simple Details Jump to Notes ] View Advanced ] Issue History ] Print ]
ID Category Severity Reproducibility Date Submitted Last Update
0000089 [GeSHi] core minor N/A 08-01-06 03:23 09-18-06 01:11
Reporter Netocrat View Status public  
Assigned To nigel
Priority normal Resolution fixed  
Status closed   Product Version 1.1.2alpha2
Summary 0000089: geshi_get_position() optimisations for regex matching
Description geshi_get_position() makes use of preg_replace() to insert the return value of microtime() into the searched string and then calls strpos() to find the location of that value. There are two issues with this:
* it's inefficient - there's no functional need to modify the string, and there's no need to iterate over it twice (i.e. once with preg_replace and once with strpos)
* there's a very small possibility that the microtime() value is already present in the string being searched.

Next, it calls preg_match_all() if $need_table was passed in as true. This is inefficienct in two ways:
1. there's no need to find all matches - the returned table only contains the first match.
2. it's duplicating the work of the first preg_replace().

These issues can all be avoided by instead using preg_split().

The attached patch does this (it also rewraps lines within the function at 80 chars wide and rewords the introductory comment).
Additional Information
Attached Files  functions.geshi.php.preg_split.patch [^] (2,603 bytes) 08-01-06 03:23

- Relationships
related to 0000058assigned nigel Analyse GeSHiCodeContext for speed optimisations 

- Notes
(0000432)
BenBE
09-13-06 00:56

Some points of your patch, especially the explicit storing of the returned value are memory-inefficient and should be left as-is. This is an issue of GeSHi already now needing alot of memory and therefore needing to reduce that memory usage.

The rest seems ok to me.
 
(0000439)
nigel
09-13-06 15:21

I'm confused on two points:

  * explicit storing of returned value - can you point out where this happens?
  * GeSHi's memory usage - I reduced it a tremendous amount when I did the parse-as-you-go method, how much memory does it actually seem to need now?

We're always going to have memory problems by the way, because the resulting string after parsing the code will always be very big.
 
(0000442)
BenBE
09-13-06 22:12

IDK if you had a look at the attached file already. There Netocrat had source like this:
$ret = array(...);
return $ret;
I just commented on this to be inefficient as the $ret will never be used for anything else and therefore can be shortened as
return array(...);
by explicit returning the array immeditly (BTW saving some parsing cycles ;-))

I'll try to do some profiling pass over GeSHi in the next few days (if I find the time) thus I can tell more about this. But I didn't look at the mem usage yet. I'll try to provide 4 values:
1. Blank PHP
2. GeSHi files included
3. GeSHI ready to parse
4. GeSHi finished parsing
I'll comment on this as neccessary.
 
(0000444)
nigel
09-13-06 23:39

Aah, I see the confusion: I was looking at the actual functions.geshi.php at CVS HEAD, which does not include this. The implementation in HEAD is actually pretty good as far as I can see.

As for memory issues, I have done no work to see how much it uses while parsing. As far as I can tell however, after a GeSHi object has been used to parse something there appears to be between 20 and 100K of claimed memory that I cannot account for. I happen to use a php accelerator which results in very small base memory usage (for whatever reason), and I haven't noticed any problems, but definately some memory-related tests will be good when we come to optimisation.
 
(0000445)
nigel
09-14-06 00:01

Marking as resolved since as far as I can tell the patch as applied works perfectly well.
 
(0000452)
nigel
09-18-06 01:11

Done.
 

- Issue History
Date Modified Username Field Change
08-01-06 03:23 Netocrat New Issue
08-01-06 03:23 Netocrat File Added: functions.geshi.php.preg_split.patch
09-13-06 00:56 BenBE Note Added: 0000432
09-13-06 00:57 BenBE Assigned To  => nigel
09-13-06 00:57 BenBE Status new => confirmed
09-13-06 12:55 BenBE Relationship added related to 0000058
09-13-06 15:21 nigel Note Added: 0000439
09-13-06 22:12 BenBE Note Added: 0000442
09-13-06 23:39 nigel Note Added: 0000444
09-14-06 00:01 nigel Note Added: 0000445
09-14-06 00:01 nigel Status confirmed => resolved
09-14-06 00:01 nigel Fixed in Version  => 1.1.2alpha3
09-14-06 00:01 nigel Resolution open => fixed
09-18-06 01:11 nigel Status resolved => closed
09-18-06 01:11 nigel Note Added: 0000452

  


Mantis 1.0.0rc2[^]
Copyright © 2000 - 2005 Mantis Group
46 total queries executed.
34 unique queries executed.
Powered by Mantis Bugtracker