[PATCH] Compressed Kernel and Modules.

[Grey]

Patch for loading compressed kernel and modules (and uncompressed too). It's accelerate system loading.
Utility "recompress" used for compressing or decompressing kernel and drivers.
It's working patch, but not perfect. Because I am newbie in Haiku and jam. For example:
1.
actions CompressFile1
{
echo "$(HAIKU_TOP)/generated/objects/linux/$(TARGET_ARCH)/release/bin/recompress/recompres $(HAIKU_TOP)/$(1) $(HAIKU_TOP)/$(1)" >> $(HAIKU_OUTPUT_DIR)/haiku.image-copy-files
}
It's terrible. But I don't know how to make this rule perfect and in style.

2. Current compressor based on Zlib. It's good choice, but not best (IMHO).
   
3. There are no special header for compressed ELF (CELF). Only zlib header used.
   
4. Compressed drivers located in /boot/system/add-ons/kernel/boot (insted symlinks).
   

... and so on...

[stippi]

Thanks a lot for the patch, nice work! I've skimmed over it to see what it does... I presume you have made some performance measurements? Can you give some numbers as to what speed-up you have achieved? As for the patch itself, besides the issues you already pointed out, I spotted some coding style violations, particularily, we put spaces around operators such as + and *. And the variable naming is always camel case, starting with lower case for local variables. Until you get feedback from other devs, you could improve that, since most of us are a bit particular about the coding style... :-) The coding style document should cover almost everything, if you are unsure about anything.

[Grey]

Replying to stippi:

... I presume you have made some performance measurements?

Can you give some numbers as to what speed-up you have achieved?

Speed-up depend of perfomance CPU and disk.
My computer - Core2 Duo P8600 2.4GHz and hard disk
HITACHI HTS542525K9SA00 5400rpm.
Raw measurements of "load kernel+modules":
Compression level 0 - 401218 CPU ticks (Syste_time() use rdtsc)
Compression level 6 - 293753 CPU ticks (Syste_time() use rdtsc)
But ticks is not real time.
Simple time estimation:
Uncompressed kernel+modules size s=1.4M
Compressed kernel+modules size S=3.9M
Delta size = S-s = 1.5M
Hard disk read speed - V=50M/s; SSD V=200M/s; USB flush V=10M/s; CD V=?
Delta time =(S-s)/V

...I spotted some coding style violations, particularily, we put spaces around operators such as + and *...

Ok. Style improved patch attached. BTW, how about using C/C++ stylers like indent, astyle or BCPP? Is there standart parameters for Haiku style?

[Grey]

Patch. Boot loading compressed kernel and drivers.

[bonefish]

Replying to Grey:

Speed-up depend of perfomance CPU and disk.
My computer - Core2 Duo P8600 2.4GHz and hard disk
HITACHI HTS542525K9SA00 5400rpm.
Raw measurements of "load kernel+modules":
Compression level 0 - 401218 CPU ticks (Syste_time() use rdtsc)
Compression level 6 - 293753 CPU ticks (Syste_time() use rdtsc)
But ticks is not real time.

If you've measured with system_time() the values are in microseconds. So that would be a speedup of a bit more than 0.1s. IMHO that alone is not really worth adding more code to the boot loader.

Simple time estimation:
Uncompressed kernel+modules size s=1.4M
Compressed kernel+modules size S=3.9M

Probably the other way around. :-)

Delta size = S-s = 1.5M
Hard disk read speed - V=50M/s; SSD V=200M/s; USB flush V=10M/s; CD V=?
Delta time =(S-s)/V

CD/DVD drives aren't that slow either wrt. continuous transfer rate, only seek times suck badly. So I wouldn't expect impressive speedups there either.

Ok. Style improved patch attached. BTW, how about using C/C++ stylers like indent, astyle or BCPP? Is there standart parameters for Haiku style?

None that I know of at least.

Some general remarks:

• As said above it doesn't seem to me that using compressing kernel and modules is worthwhile. Before you continue, I'd wait for one or two other kernel developers to give an opinion.
• I would find it more elegant, if all modules were compressed, not only the boot modules. That would avoid different treatment of boot and non-boot modules and would be necessary anyway, if we decide to select the boot modules algorithmically at boot time (IIRC that was at least an idea quite a while ago). The downside of using compressed ELF files is that we don't support resources for those yet and it won't be easy to do that either. Doesn't matter all that much, though.
• Why the recompress tool? Simply use gzip I'd say.

Some comments regarding the patch itself. I'll point out coding style violations only exemplarily.

As you've mentioned yourself things in HaikuImage and ImageRules really need to be done differently. If we decide to accept the patch, I'd fix that.

===================================================================
--- src/system/boot/loader/elf.cpp	(revision 34680)
+++ src/system/boot/loader/elf.cpp	(working copy)
@@ -16,6 +16,7 @@
 #include <unistd.h>
 #include <string.h>
 #include <stdlib.h>
+#include <zlib.h>

 //#define TRACE_ELF
 #ifdef TRACE_ELF
@@ -28,6 +29,67 @@
 static bool sLoadElfSymbols = false;


+/* Decompress from file fd to memory dest until EOF.
+   inflater() returns B_OK on success, B_NO_MEMORY if memory could not be
+   allocated for processing, B_BAD_TYPE if the deflate data is
+   invalid or incomplete.
+*/

That should be doxygen style comment directly above the function.

+#define CHUNK (16 * 1024)
+
+static status_t
+inflater(int fd, char *dest, int dest_size)

Parameters and local variables are camel case.

+{
+    char in[CHUNK];

In boot loader and kernel the stack is limited. That should be either allocated on the heap or go into the BSS.

@@ -119,7 +181,7 @@


 static status_t
-load_elf_symbol_table(int fd, preloaded_image *image)
+load_elf_symbol_table(int fd, preloaded_image *image, char *unzip_buf)

As already mentioned: camel case variables. Also: descriptive identifier names are preferred, i.e. "unzipBuffer" this case.

@@ -137,12 +199,7 @@
 		return B_NO_MEMORY;
 	}

-	ssize_t length = read_pos(fd, elfHeader.e_shoff, sectionHeaders, size);
-	if (length < size) {
-		TRACE(("error reading in program headers\n"));
-		status = B_ERROR;
-		goto error1;
-	}
+	memcpy(sectionHeaders, unzip_buf+elfHeader.e_shoff, size);

You're not doing the length sanity check anymore (several occurrences). Spaces around the operator.

@@ -221,19 +265,17 @@


 status_t
-elf_load_image(int fd, preloaded_image *image)
+elf_load_image(int fd, preloaded_image *image, char *unzip_buf, int unzip_buf_size)

80 columns limit.

 {
 	size_t totalSize;
 	status_t status;

 	TRACE(("elf_load_image(fd = %d, image = %p)\n", fd, image));

+	inflater(fd, unzip_buf, unzip_buf_size );

Incorrect space before the ')'.

@@ -361,14 +396,9 @@

 		TRACE(("load segment %d (%ld bytes)...\n", i, header.p_filesz));

-		length = read_pos(fd, header.p_offset,
-			(void *)(region->start + (header.p_vaddr % B_PAGE_SIZE)),
-			header.p_filesz);
-		if (length < (ssize_t)header.p_filesz) {
-			status = B_BAD_DATA;
-			dprintf("error reading in seg %d\n", i);
-			goto error2;
-		}
+		memcpy((void *)(region->start + (header.p_vaddr % B_PAGE_SIZE)),
+				unzip_buf + header.p_offset,
+				header.p_filesz);

Indentation only by a single additional tab.

===================================================================
--- src/system/boot/loader/main.cpp	(revision 34680)
+++ src/system/boot/loader/main.cpp	(working copy)
@@ -24,6 +24,8 @@
 #	define TRACE(x) ;
 #endif

+#define KERNEL_BUF_SIZE (4*1024*1024)
+#define UNZIP_BUF_SIZE  (4*1024*1024)

 extern "C" int
 main(stage2_args *args)
@@ -76,8 +78,17 @@

 	if (volume != NULL) {
 		// we got a volume to boot from!
+
+		// allocate upper memory for uncompress buffer
+		char *kernel_buf = (char *)kernel_args_malloc(KERNEL_BUF_SIZE);
+		char *unzip_buf = (char *)kernel_args_malloc(UNZIP_BUF_SIZE);
+		kernel_args_free(kernel_buf);

This deserves at least a comment. I suppose you're trying to reserve the range for the kernel code.

+		if (!unzip_buf) {
+			panic("Could not allocate memory for uncompressing kernel!\n" );
+		}

If "if" condition and statement are single line only, the braces should be omitted.

Index: src/bin/recompress/recompress.c
===================================================================
--- src/bin/recompress/recompress.c	(revision 0)
+++ src/bin/recompress/recompress.c	(revision 0)
@@ -0,0 +1,70 @@
+/*! Simple compress/uncompress. Use ZLIB */
+#include <stdio.h>

Missing copyright/license header.

Index: src/bin/recompress/Jamfile
===================================================================
--- src/bin/recompress/Jamfile	(revision 0)
+++ src/bin/recompress/Jamfile	(revision 0)
@@ -0,0 +1,46 @@
+SubDir HAIKU_TOP src bin recompress ;
+
+SetSubDirSupportedPlatformsBeOSCompatible ;

Unnecessary.

+Objects $(common_files) $(common_files2) $(util_files) ;

Looks like a left-over.

+UseLibraryHeaders zlib ;
+SEARCH_SOURCE += [ FDirName $(HAIKU_TOP) src libs zlib ] ;
+
+SubDirCcFlags -DNOID -DHAVE_STDINT_H=0 ;

Rather add macro definitions by adding them to the DEFINES variable. Also: why "=0"? Should be "=1", I guess.

+SubDirSysHdrs [ FDirName $(SUBDIR) include ] ;
+SubDirSysHdrs [ FDirName $(SUBDIR) lib ] ;

Superfluous. You don't have "include" and "lib" subdirectories anyway.

+Depends recompress : recompres ;

Nope.

+BinCommand recompress :
+	recompress.c
+	compress.c
+	uncompr.c
+	deflate.c
+	crc32.c
+	adler32.c
+	inflate.c
+	zutil.c
+	trees.c
+	inffast.c
+	inftrees.c
+	: be
+;

Linking against libbe.so not necessary. Also, just link against "z" instead of compiling in the zlib sources.

+BuildPlatformMain recompres :

Build tools should be built in src/tools and the target name in this case should be "<build>recompress".

[axeld]

I'm all with Ingo here. While compressed ELF certainly had its uses in the embedded world, even there the storage restrictions are usually not that hard anymore.

However, I wouldn't mind CELF support, but then it really should be complete, ie. all binaries should be compressable.

[Grey]

Replying to bonefish:
Thanks a lot for the advices about coding style! It's very useful. Add to Coding Guidelines, please.

Replying to bonefish:

Replying to Grey:

• As said above it doesn't seem to me that using compressing kernel and modules is worthwhile....
  

It seems to me too. And I don't understand why linux use compressed kernel.

Replying to axeld:

However, I wouldn't mind CELF support, but then it really should be complete, ie. all binaries should be compressable.

I'll try to find more effective way to speed-up booting.

[stippi]

To be honest, I am quite happy with Haiku booting times. Once it supports standby and hibernate, it should be perfect. Is Haiku booting too slow on your hardware for some reason?

[Grey]

Replying to stippi:

To be honest, I am quite happy with Haiku booting times. Once it supports standby and hibernate, it should be perfect. Is Haiku booting too slow on your hardware for some reason?

I am not quite happy with Haiku booting time. Haiku booting is very fast whit compare to Win, faster then Linux, but slower then Menuet OS or Kolibri OS. Last OS show that it's possible to speed-up boot.
From marketing point of view: It's very nice, if Haiku loading from USB disk or from Live CD will be faster then Win loading from hard disk.

About my hardware: sometimes Haiku fails to boot on my Tachpad T400. Reason - problems with AHCI. I am writing patch now...

[bonefish]

Replying to Grey:

Replying to stippi:

To be honest, I am quite happy with Haiku booting times. Once it supports standby and hibernate, it should be perfect. Is Haiku booting too slow on your hardware for some reason?

I am not quite happy with Haiku booting time. Haiku booting is very fast whit compare to Win, faster then Linux, but slower then Menuet OS or Kolibri OS. Last OS show that it's possible to speed-up boot.

I had a look at KolibriOS and it's obviously not nearly as complete as Haiku. E.g. this is a complete listing of the "Drivers" directory:

Atikms Atikms.dll Vmode.mdr Com_mouse.obj Infinity.obj
Ps2mouse.obj Sb16.obj Sis.obj Sound.obj

Quite a bit of Haiku's boot time can be attributed to driver/hardware recognition and initialization. The USB stuff alone takes several seconds on my machine. Also the various services started in Haiku's Bootscript don't seem to have a comparable equivalent in KolibriOS. So comparing boot times is not really fair.

I failed to run MenuetOS boot floppy and CD image under qemu, so I can't really comment on it.

Anyway, I guess the main points for boot time optimization are (1) more intelligent I/O (e.g. precaching) and (2) choosing drivers for hardware more directly (e.g. by PCI device ID) instead of trying all of them. Furthermore it would be great, if it was possible to recognize earlier whether the boot volume requires USB and postpone USB initialization if not.

[Grey]

Replying to bonefish:

... Quite a bit of Haiku's boot time can be attributed to driver/hardware recognition and initialization. The USB stuff alone takes several seconds on my machine. ...

Is there are any profiling software for Haiku? How about adding time information to syslog?

Anyway, I guess the main points for boot time optimization are (1) more intelligent I/O (e.g. precaching) and (2) choosing drivers for hardware more directly (e.g. by PCI device ID) instead of trying all of them. Furthermore it would be great, if it was possible to recognize earlier whether the boot volume requires USB and postpone USB initialization if not.

There is one more way for boot time optimization: ​http://dev.haiku-os.org/ticket/5163 :)

[tqh]

While I was fixing ACPI I rebooted a lot. I realized quite early that everything I could do to speed up boot-time would get me more productive in fixing issues. So if there are clean ways to speed up booting I'm all for it, even if it's just for those devs that need to reboot a lot..

Btw, with SSD's you usually turn of precaching as it slows down booting.

[axeld]

There is profiling software for Haiku, also for the boot process. For normal software, the "profile" command can be used (the KCachegrind output is definitely worth a look).

For the boot process, you have to call the "start_system_profiler()" (see src/system/kernel/debug/system_profiler.cpp) method somewhere in main(), and then later use profile to retrieve the collected data.