Re: Blocking DNS lookup on MC startup
Hello Pavel, On Wed, 2006-05-31 at 11:53 +0300, Pavel Tsekov wrote: It is clear that this happens only of smbfs support is enabled. Why? Because you can only reproduce it once you enable smbfs? I see several solutions to this problem: 1) The user arranges for her/his network setup to be correct That's always a good idea, but I've heard sourceforge being mentioned multiple times in relation to this. I think sf users don't have much influence on the configuration of those systems. 2) We fix get_myname() so that it won't perform lookups except when asked to do so Moving the Get_Hostbyname() call to inside the if(ip) seems a proper solution. 3) We remove the call to get_myname() from smbfs_init() That would assume ip always is NULL. Do you have any indication that it is? Rather curious by the way that the only call to get_myname passes NULL for my_name, but that could be well be caused by the fact that we only partially include the samba files. Leonard. -- mount -t life -o ro /dev/dna /genetic/research ___ Mc-devel mailing list http://mail.gnome.org/mailman/listinfo/mc-devel
Re: Blocking DNS lookup on MC startup
On Wed, 31 May 2006, Leonard den Ottolander wrote: On Wed, 2006-05-31 at 11:53 +0300, Pavel Tsekov wrote: It is clear that this happens only of smbfs support is enabled. Why? Because you can only reproduce it once you enable smbfs? 1) The backtrace shows it. 2) If i had smbfs disabled (and I do) i wasn't able to reproduce the slowdown. 3) There are not gethostbyname() during MC startup. I see several solutions to this problem: 1) The user arranges for her/his network setup to be correct That's always a good idea, but I've heard sourceforge being mentioned multiple times in relation to this. I think sf users don't have much influence on the configuration of those systems. It has confirmed that this is not the reason for the slowdown on sf. Please, read the whole thread. 2) We fix get_myname() so that it won't perform lookups except when asked to do so Moving the Get_Hostbyname() call to inside the if(ip) seems a proper solution. Yep. Maybe that's what we want to do. The impact will be minor. ___ Mc-devel mailing list http://mail.gnome.org/mailman/listinfo/mc-devel
Re: Blocking DNS lookup on MC startup
Hi Pavel, On Wed, 2006-05-31 at 14:49 +0300, Pavel Tsekov wrote: 1) The backtrace shows it. Yes, but that doesn't mean it couldn't hang elsewhere. Hence my question. 2) If i had smbfs disabled (and I do) i wasn't able to reproduce the slowdown. Right. 3) There are not gethostbyname() during MC startup. I see now. It's used in ftpfs, but only on the instantiation of a socket. It has confirmed that this is not the reason for the slowdown on sf. Please, read the whole thread. O yeah, I read and forgot ;) . Sorry. Moving the Get_Hostbyname() call to inside the if(ip) seems a proper solution. Yep. Maybe that's what we want to do. The impact will be minor. Good. Leonard. -- mount -t life -o ro /dev/dna /genetic/research ___ Mc-devel mailing list http://mail.gnome.org/mailman/listinfo/mc-devel
Re: Blocking DNS lookup on MC startup
On Wed, 31 May 2006, Leonard den Ottolander wrote: On Wed, 2006-05-31 at 14:49 +0300, Pavel Tsekov wrote: 1) The backtrace shows it. Yes, but that doesn't mean it couldn't hang elsewhere. Hence my question. I think, but I am not sure, that the rest of the gethostbyname() and related calls are interruptable. I remember that Arpie submitted a patch for gethostbyname() in ftpfs no so long ago. Moving the Get_Hostbyname() call to inside the if(ip) seems a proper solution. Yep. Maybe that's what we want to do. The impact will be minor. Good. I've notified Andrew Samoilov about this discussion. I suggest that we wait for his opinion on this matter. ___ Mc-devel mailing list http://mail.gnome.org/mailman/listinfo/mc-devel
Re: Blocking DNS lookup on MC startup
Hi, On Wed, 2006-05-31 at 15:26 +0300, Pavel Tsekov wrote: I've notified Andrew Samoilov about this discussion. I suggest that we wait for his opinion on this matter. Ok. By the way, if neither my_name nor ip are set we can safely return before the call to gethostname(). Leonard. -- mount -t life -o ro /dev/dna /genetic/research ___ Mc-devel mailing list http://mail.gnome.org/mailman/listinfo/mc-devel
Re: [RFE][PATCH] Display free space on device in panels
Hi Pavel,
On Tue, 2006-05-30 at 14:56 +0300, Pavel Tsekov wrote:
On Mon, 29 May 2006, Jindrich Novy wrote:
On Fri, 2006-05-19 at 14:29 -0400, Pavel Roskin wrote:
Can we avoid any highlighting? I think Far Manager does it right:
http://red-bean.com/proski/mc/far.png
Attaching the patch without highlighting. The main changes are that I
moved the show_free_space() into main.c since screen.c lacks a header
file and a call is needed from cmd.c to minimize frequency of gathering
filesys info. The free space widget isn't displayed at all when browsing
a non-local fs. Filesystem statistics are updated immediately when mc
requests a reread (ctrl-r is pressed).
Please review, some further mistakes are possible.
Now, after I reviewed the patch for a second time I am pretty convinced
that the free space info should be displayed in the mini status area
and not on its upper frame i.e. as it is in FAR manager (what Pavel
Rosking suggested).
Yes, otherwise it would be too little space for cwd, especially for
80x25 terminals.
MC tries to optimize the screen drawing by updating only certain parts
of the screen (the directory contents, the mini status) when not otherwise
necessary. Simply put MC doesn't update the whole screen when you go down
one file in the panel, when you change a directory, etc. With the current
patch you actually break that optimization by inserting a call to
mini_info_separator() in show_dir() which is called each time you move
through the file list for example.
Ok, the only speed related optimization I see is to not to draw the free
space widget every time the show_free_space() is called, but only in
case when old_cwd and panel_cwd differs. The trick to force redraw when
user presses ctrl-r by setting old_cwd to NULL will work then anyway.
Your patch may be modified to so that it will work properly i.e. it wont
break the optimization but it becomes ugly (codewise). It may be modified
to be not so ugly by sacrificing the optimization a bit i.e. put
a call to mini_info_separator () in panel_update_contents(). It also
can be made to fit into the current scheme by making the free space info
part of the mini status area.
Did you do some profiling that it slows mc down significantly? I haven't
seen a noticable delay, especially after show_free_space() doesn't draw
the widget each time it's called.
Now, there are some other small issues with the patch as-is:
--- mc-4.6.1a/src/main.c.showfree 2006-05-29 12:41:36.0 +0200
+++ mc-4.6.1a/src/main.c 2006-05-29 13:04:50.0 +0200
@@ -402,6 +409,8 @@
int reload_other = !(force_update UP_ONLY_CURRENT);
WPanel *panel;
+show_free_space(current_panel);
+
update_one_panel (get_current_index (), force_update, current_file);
if (reload_other)
update_one_panel (get_other_index (), force_update, UP_KEEPSEL);
This part is not necessary. And it will draw the free_space_info() even
if there is no mini status (though you won't see it).
show_free_space() checks whether free_space != 0 otherwise it won't draw
anything.
@@ -467,6 +476,37 @@
}
}
+void
+show_free_space(WPanel *panel)
+{
+struct stat st;
This is not necessary as well as the code which fills it.
Yes, I'll remove it.
Jindrich
___
Mc-devel mailing list
http://mail.gnome.org/mailman/listinfo/mc-devel
Re: [RFE][PATCH] Display free space on device in panels
On Wed, 31 May 2006, Jindrich Novy wrote: On Tue, 2006-05-30 at 14:56 +0300, Pavel Tsekov wrote: On Mon, 29 May 2006, Jindrich Novy wrote: On Fri, 2006-05-19 at 14:29 -0400, Pavel Roskin wrote: Can we avoid any highlighting? I think Far Manager does it right: http://red-bean.com/proski/mc/far.png Attaching the patch without highlighting. The main changes are that I moved the show_free_space() into main.c since screen.c lacks a header file and a call is needed from cmd.c to minimize frequency of gathering filesys info. The free space widget isn't displayed at all when browsing a non-local fs. Filesystem statistics are updated immediately when mc requests a reread (ctrl-r is pressed). Please review, some further mistakes are possible. Now, after I reviewed the patch for a second time I am pretty convinced that the free space info should be displayed in the mini status area and not on its upper frame i.e. as it is in FAR manager (what Pavel Rosking suggested). Yes, otherwise it would be too little space for cwd, especially for 80x25 terminals. Well, maybe I didn't explain well - I mean't the horizontal line between the mini status and the directory listing. MC tries to optimize the screen drawing by updating only certain parts of the screen (the directory contents, the mini status) when not otherwise necessary. Simply put MC doesn't update the whole screen when you go down one file in the panel, when you change a directory, etc. With the current patch you actually break that optimization by inserting a call to mini_info_separator() in show_dir() which is called each time you move through the file list for example. Ok, the only speed related optimization I see is to not to draw the free space widget every time the show_free_space() is called, but only in case when old_cwd and panel_cwd differs. The trick to force redraw when user presses ctrl-r by setting old_cwd to NULL will work then anyway. Maybe this wasn't clear as well - I'll try by pointing you directly to the code. See the difference between paint_panel () and panel_update_contents(). Your patch may be modified to so that it will work properly i.e. it wont break the optimization but it becomes ugly (codewise). It may be modified to be not so ugly by sacrificing the optimization a bit i.e. put a call to mini_info_separator () in panel_update_contents(). It also can be made to fit into the current scheme by making the free space info part of the mini status area. Did you do some profiling that it slows mc down significantly? I haven't seen a noticable delay, especially after show_free_space() doesn't draw the widget each time it's called. There is no need to profile MC. It is not speedup in terms of CPU cycles but in reduced screen update i.e. why paint the frame each time if it really doesn't change at all. Now, there are some other small issues with the patch as-is: --- mc-4.6.1a/src/main.c.showfree 2006-05-29 12:41:36.0 +0200 +++ mc-4.6.1a/src/main.c2006-05-29 13:04:50.0 +0200 @@ -402,6 +409,8 @@ int reload_other = !(force_update UP_ONLY_CURRENT); WPanel *panel; +show_free_space(current_panel); + update_one_panel (get_current_index (), force_update, current_file); if (reload_other) update_one_panel (get_other_index (), force_update, UP_KEEPSEL); This part is not necessary. And it will draw the free_space_info() even if there is no mini status (though you won't see it). show_free_space() checks whether free_space != 0 otherwise it won't draw anything. Yes. But it will draw when the mini status is disabled. If you are not convinced - you could try stepping trough the body of update_panels() with a debugger. This slows the execution time and it will allow you to see the free space info being printed. ___ Mc-devel mailing list http://mail.gnome.org/mailman/listinfo/mc-devel
Re: [RFE][PATCH] Display free space on device in panels
On Wed, 31 May 2006, Oswald Buddenhagen wrote: On Wed, May 31, 2006 at 05:13:29PM +0300, Pavel Tsekov wrote: There is no need to profile MC. It is not speedup in terms of CPU cycles but in reduced screen update i.e. why paint the frame each time if it really doesn't change at all. it should be pointed out that the screen libs (ncurses and slang) optimize away redundant paints (this can be best proved by messing up the screen (e.g., with write) and doing something that _certainly_ does a full paint - like opening the editor). so the redundant painting happens only in the screen lib's frame buffer, which isn't that expensive, really. so if it's only one line and the optimization would be pretty complicated, it simply would not pay. but i can't judge that case, as i didn't read the code. It's there and has been for some time. The new free space patch is simply not doing the right thing with respect to the existing code. Whether this optimization is worth or not is something that should be discussed in another thread. But if anyone feels that the new patch should go in as is lets put it for a vote. It is really not my intention to stop that patch from being checked in. As for the smart screen libraries - yes they do try to reduce the number of real screen updates. But this suggestion that I made to Jindrich i.e. to trace with gdb: http://mail.gnome.org/archives/mc-devel/2006-May/msg00119.html It shows that S-Lang is not doing the smartest thing. There are many other examples when data is printed to the screen right away. ___ Mc-devel mailing list http://mail.gnome.org/mailman/listinfo/mc-devel
Re: Blocking DNS lookup on MC startup
On Wed, 31 May 2006, Pavel Tsekov wrote: On Wed, 31 May 2006, Pavel Tsekov wrote: On Tue, 30 May 2006, Pavel Tsekov wrote: Yes. I understood that the first time through. There was a similiar bug report already. The question is: why MC is doing DNS queries? Is possible to disable them? Maybe someone else is doing those queries. A library linked to MC most likely. I've tried to reproduce that many times and I cannot get neither the slowdown nor those particualr dns lookups for interface names. I disconnect the network cable and still I am not able to get the described behaviour. Ok, I was able to reproduce it - it seems that the slowdown happens only if MC has smbfs support. I'll investigate. There are more calls to gethostbyname() after the get_myname () call. load_interfaces() is doing gethostbyname() for every single value of the interfaces parameter of smb.conf. If interfaces doesn't contain an ip_address/netmask pair load_interfaces() tries to resolve the value via gethostbyname() :( ___ Mc-devel mailing list http://mail.gnome.org/mailman/listinfo/mc-devel
