Advanced search  

News:

cpg1.5.48 Security release - upgrade mandatory!
The Coppermine development team is releasing a security update for Coppermine in order to counter a recently discovered vulnerability. It is important that all users who run version cpg1.5.46 or older update to this latest version as soon as possible.
[more]

Pages: [1]   Go Down

Author Topic: [Done]: [Suggestion for code review] get rid of the slow ereg(i) and ereg_replace  (Read 26979 times)

0 Members and 1 Guest are viewing this topic.

ripat

  • Coppermine newbie
  • Offline Offline
  • Posts: 11
    • Regex tester

Hi,

As I mentioned in another post the ereg(i) functions are notoriously slow and greedy. The can easily be replaced by the PCRE preg_* functions or regular string functions like strpos or str_replace.

This morning, I went trough all files and found a number of occurrences of ereg functions. Here follows my suggestions for replacement. They were all tested for speed. Where my comments says Speed factor +8, it means that my suggestion for replacement is 8 times faster. All speed test were done on loops of 200 iterations.

http://www.lumadis.be/jeanluc/vault/ereg.txt

I still need to go through the following files:
/include/functions.inc.php
/include/picmgmt.inc.php
/include/imageObjectIM.class.php
/lang/dutch.php
/profile.php
/ratepic.php
/register.php
/report_file.php
/upload.php

I'll continue if this suggestion is found useful.

A+
JL
« Last Edit: November 30, 2008, 12:33:31 am by Nibbler »
Logged

Joachim Müller

  • Dev Team member
  • Coppermine addict
  • ****
  • Offline Offline
  • Gender: Male
  • Posts: 47843
  • aka "GauGau"
    • gaugau.de

Yes, I indeed find your suggestions very usefull; I'm looking forward to your suggestions, especially for include/functions.inc.php (as this file contains some crude replacement constructs that have been a thorn in coppermine's side for long.) However, could I suggest adding comments to the non-trivial regular expressions to enable devs and power-users who are not so familiar with regex syntax to come up with possible future edits easily? Having efficient code is great, but for a community effort like coppermine, it's mandatory to enable others to understand the code as well. I consider it one of the defficiencies of coppermine that there is little developer documentation available.

Once again: thank you very much for your suggestions.

Joachim

P.S. As you certainly know your way around, I'd like to encourage you to look at the devel code instead of looking at the code of the stable branch - the devel code is more advanced and it's the code that will actually get improved; for the current stable (cpg1.4.x), only bugfixes will be applied, but no code improvements or additional features. Please refer to the preliminary subversion documentation for cpg1.5.x (under construction). Additional stuff: http://sourceforge.net/svn/?group_id=89658
Folder to check out: https://coppermine.svn.sourceforge.net/svnroot/coppermine/trunk/cpg1.5.x/ (SVN client), http://coppermine.svn.sourceforge.net/viewvc/coppermine/trunk/cpg1.5.x/ (web-SVN)
« Last Edit: July 11, 2007, 07:21:36 pm by GauGau »
Logged

ripat

  • Coppermine newbie
  • Offline Offline
  • Posts: 11
    • Regex tester

hi Johachim,

I have never worked on a collaborative project but I am ready to give it a try. However, I will need some coaching on how to use subversion. The tigris.org doc's and faq are more svn server oriented.

I have downloaded the svn client for linux and I am presently working on the regex functions used in v1.5.

Should I make the changes directly on my local v1.5 files - with the necessary comments. How to upload them back to your repository without making any damage?

A+
JL
Logged

Abbas Ali

  • Administrator
  • Coppermine addict
  • *****
  • Country: in
  • Offline Offline
  • Gender: Male
  • Posts: 2165
  • Spread the PHP Web
    • Ranium Systems

You can post your code changes here or give us the patch and we will do the needful. You cannot directly upload/commit in the repository.
Logged
Chief Geek at Ranium Systems

ripat

  • Coppermine newbie
  • Offline Offline
  • Posts: 11
    • Regex tester

OK. There are quite a number of changes. That will be a long post.

Is it OK with this link?
http://www.lumadis.be/jeanluc/vault/cpg15x_txt.php

I will review all the functions tomorrow.

A+
JL
Logged

Joachim Müller

  • Dev Team member
  • Coppermine addict
  • ****
  • Offline Offline
  • Gender: Male
  • Posts: 47843
  • aka "GauGau"
    • gaugau.de

OK. There are quite a number of changes. That will be a long post.
Doesn't matter - as long as there is good content, we don't mind long postings. And yours is good and valid content :)

Is it OK with this link?
That's OK just as well. Whatever option you prefer.

Joachim
Logged

ripat

  • Coppermine newbie
  • Offline Offline
  • Posts: 11
    • Regex tester

Well I went through all 1.5.x files that contained ereg() functions and made the changes accordingly. I skipped all files from directory include/.svn/text-base/ that seem to be duplicated from main include/

All changes are here:
text mode: http://www.lumadis.be/jeanluc/vault/cpg15x_txt.php
or
colorized: http://www.lumadis.be/jeanluc/vault/cpg15x_col.php

I still have to check a couple of things. Should be done tomorrow. Also, I have no test script to test the impact of the changes on the application as a whole. It should be no problem as it is quite straight forward to replace ereg by preg_match. Optimization of the pattern for speed is another story. Took me some time...

Of course when I quote a speed gain of, say 10 times faster, it is for the block of code that contains the ereg() function - not for the whole script. But as we say over here "les petits ruisseaux font les grandes rivières" (small streams make the large rivers).

A+
JL
Logged

Nibbler

  • Guest
Re: [Suggestion for code review] get rid of the slow ereg(i) and ereg_replace
« Reply #7 on: November 30, 2008, 12:33:19 am »

Committed to 1.5.
Logged
Pages: [1]   Go Up
 

Page created in 0.019 seconds with 18 queries.