From mboxrd@z Thu Jan 1 00:00:00 1970 From: Michael Tremer To: development@lists.ipfire.org Subject: Re: [PATCH] speed.cgi: reduce system load by copying two general-functions. Date: Mon, 25 Oct 2021 19:54:18 +0100 Message-ID: In-Reply-To: MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="===============0565255368841793312==" List-Id: --===============0565255368841793312== Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable Hello, > On 22 Oct 2021, at 11:30, Bernhard Bitsch wrote: >=20 > Hi Michael, >=20 > you are right from a formal sight. > But testing the patch shows a reduction of system load in my system ( ~50% = ). This is insane that loading a Perl module takes that many resources. > So the impact is not neglectable. I didn=E2=80=99t dispute that :) > Because we should not copy functions, as stated, we must review these Perl = modules. This will be an ongoing process. I fear, we will find some copys sca= ttered in various files. > One thing I noticed while reading the various sources, is the relation of g= eneral-functions.pl and network-functions.pl. general-functions.pl includes t= he latter ( everytime ) and contains also network related functions. Maybe th= is is a start point for brushing up. Yes, generally it would be good to have functions where they should be. Unfor= tunately there are lots of =E2=80=9Cwrong=E2=80=9D implementations that behav= e a certain way and the code that calls them does unexpected stuff when a fun= ction returns something else. This is a large mess and I would like to avoid making it any worse. -Michael >=20 > Regards, > Bernhard >=20 >=20 > Am 22.10.2021 um 12:07 schrieb Michael Tremer: >> Hello Arne, >> I do not think I am happy with this patch because it duplicates code. >> Whenever we change or fix anything in any of those copied functions, we *w= ill* forget the copy in speed.cgi. >> What is the reason we cannot load general-funcions.pl in speed.cgi? >> Loading a slightly larger Perl file like general-functions.pl cannot impac= t the performance of the interpreter. If it does, the way should be to fix an= y slow parts in it. >> -Michael >>> On 22 Oct 2021, at 09:05, Arne Fitzenreiter wrote: >>>=20 >>> include general-functions.pl load and initialize many subfunctions that a= re not >>> needed by speed.cgi which was executed very ofthen. >>> So this reduce the system load significant if webif was open in browser >>> and ajax-speed display enabled. >>>=20 >>> Signed-off-by: Arne Fitzenreiter >>> --- >>> config/cfgroot/general-functions.pl | 2 ++ >>> html/cgi-bin/speed.cgi | 46 +++++++++++++++++++++++++++-- >>> 2 files changed, 45 insertions(+), 3 deletions(-) >>>=20 >>> diff --git a/config/cfgroot/general-functions.pl b/config/cfgroot/general= -functions.pl >>> index f72d6588c..c10a04faa 100644 >>> --- a/config/cfgroot/general-functions.pl >>> +++ b/config/cfgroot/general-functions.pl >>> @@ -53,6 +53,7 @@ sub system_background($) { >>> } >>>=20 >>> # Returns the output of a shell command >>> +# if you change this also check speed.cgi that include a local copy for = systemload reasons >>> sub system_output($) { >>> my @command =3D @_; >>> my $pid; >>> @@ -1227,6 +1228,7 @@ sub firewall_reload() { >>> } >>>=20 >>> # Function which will return the used interface for the red network zone = (red0, ppp0, etc). >>> +# if you change this also check speed.cgi that include a local copy for = systemload reasons >>> sub get_red_interface() { >>>=20 >>> open(IFACE, "${General::swroot}/red/iface") or die "Could not open /var/= ipfire/red/iface"; >>> diff --git a/html/cgi-bin/speed.cgi b/html/cgi-bin/speed.cgi >>> index b550fda52..651c3c0b3 100644 >>> --- a/html/cgi-bin/speed.cgi >>> +++ b/html/cgi-bin/speed.cgi >>> @@ -19,7 +19,47 @@ >>> # = # >>> #########################################################################= ###### >>>=20 >>> -require '/var/ipfire/general-functions.pl'; >>> +########################################################################= ####### >>> +# functions copied from general-functions.pl for speed improvement becau= se # >>> +# loading and initializing the whole general-functions.pl every second c= reate # >>> +# high system load = # >>> +###########################################OA###########################= ####### >>> +# >>> +# Returns the output of a shell command >>> +sub General__system_output($) { >>> + my @command =3D @_; >>> + my $pid; >>> + my @output =3D (); >>> + >>> + unless ($pid =3D open(OUTPUT, "-|")) { >>> + open(STDERR, ">&STDOUT"); >>> + exec { ${command[0]} } @command; >>> + die "Could not execute @command: $!"; >>> + } >>> + >>> + waitpid($pid, 0); >>> + >>> + while () { >>> + push(@output, $_); >>> + } >>> + >>> + close(OUTPUT); >>> + return @output; >>> +} >>> +# >>> +# Function which will return the used interface for the red network zone= (red0, ppp0, etc). >>> +sub General__get_red_interface() { >>> + >>> + open(IFACE, "/var/ipfire/red/iface") or die "Could not open /var/ip= fire/red/iface"; >>> + >>> + my $interface =3D ; >>> + close(IFACE); >>> + chomp $interface; >>> + >>> + return $interface; >>> +} >>> +# >>> +########################################################################= ####### >>>=20 >>> my $data_last =3D $ENV{'QUERY_STRING'}; >>> my $rxb_last =3D 0; >>> @@ -38,8 +78,8 @@ foreach $field (@fields) { >>> } >>> } >>>=20 >>> -my $interface =3D &General::get_red_interface(); >>> -my @data_now =3D &General::system_output("ip", "-s", "link", "show", "$i= nterface"); >>> +my $interface =3D &General__get_red_interface(); >>> +my @data_now =3D &General__system_output("ip", "-s", "link", "show", "$i= nterface"); >>>=20 >>> my $lastline; >>> my $rxb_now =3D 0; >>> --=20 >>> 2.25.1 >>>=20 --===============0565255368841793312==--