> From: ion(a)svn.reactos.org
>
> * Do not pollute the kernel with 10 real-time threads and 5
> high-priority threads in order to manage work items. Work
> threads are very-low priority (< 7) and should never pre-empt
> userthreads like they do now. 1 priority 7, 5 priority 5 and
> 3 priority 4 threads are now properly created.
I haven't looked at your code yet, but the comment worries me. Does it mean
that if a usermode app is stuck in a "while (1) ;" loop stuff queued by
IoAllocateWorkItem( ) never gets executed? If I misunderstood, nevermind,
disregard this message. If this is true we're in big trouble. We're using
work items in a lot of places to get from DPC level to PASSIVE level. For
example, the networking stack queues a work item when data was received from
the network card. I'd hate to see a stuck usermode app halt all network
communications...
GvG
Hi,
I have come to the conclusion that using -O2 is beneficial even for DBG
= 1 builds, and that it should be set on by default on all builds. The
typically given reason for not using optimizations on a "Debug" build is
because these apparently make assembly code harder to read. I have
realized otherwise, and as seen in the example that I will include
below, I'm sure this will be mutually agreed on. I note the following
advantages in using -O2 on a DBG = 1 build as well:
- -O2 makes the compiler do additional checks. For example, gcc will NOT
detect uninitialized variables unless -O2 is being used, even though
they are a very important programming bug. Apart from finding more bugs,
it also makes trunk compilable. Right now, I see at least two commits by
Thomas or others being made every week in order to fix some code which
used unitinialized variables (I myself have been guilty of this). This
means that some of us, like Thomas, have to constantly fix other
people's mistakes.
- -O2 means less last-minute blockers. Because we release in -O2 but
almost never build it like that, this creates a big problem for people
like Andrew or Brandon, which handle the release process and do testing.
Because the -O2 build gets less testing coverage, it is very possible
for a critical bug to be in ROS for a month before anyone notices it at
release time, in which case we will all have to scramble to find a fix
for it.
- -O2 will not undefine DBG or change anything else in the code. All the
advatanges, extra error checking and assertions of the DBG =1 build
would remain.
- -O2 builds are much faster, greatly helping testing speed.
- -O2 builds are much more likely to bring up race conditions and other
important timing bugs we need to watch out for.
- -O2 means easier debugging. This point is really important because
until I realized how true it was, I didn't want to bring this up. Here
is a pseudo(but real) disassembly of something I've seen in my dbg = 1
kernel binary while debugging:
0x40b845:
push ebp
mov ebp, esp
sub esp, 4
mov [ebp-4], fs:18h
mov eax, [ebp-4]
leave
retn
0x4bc8a5:
push ebp
mov ebp, esp
sub esp, 4
call 0x40b845
mov ecx, [eax+1c]
mov [ebp-4], eax
mov eax, [ebp-4]
leave
retn
0x42b845:
push ebp
mov ebp, esp
sub esp, 4
call 0x4bc8a5
mov ecx, [eax+124]
mov [ebp-4], eax
mov eax, [ebp-4]
leave
retn
KeFooBar:
push ebp
mov ebp, esp
sub esp, 4c
call 0x42b845
mov [ebp-0xc], eax
mov eax, [ebp-0xc]
<..>
leave
retn
This is how it looks with -O2
KeFooBar:
push ebp
mov ebp, esp
sub esp, 4c
mov eax, fs:124h
<..>
leave
retn
I hope we can all agree on which one of these is readable. The -O2 build
clearly shows you that eax is fs:124h, which you oughta know is
Pcrb->CurrentThread; even if you don't, you can easily check in a
header. The non-o2 build calls 3 other functions, out of which 2 are
merely calling other functions themselves (due to lack of symbols you
have no way of knowing what these functions are doing), until we finally
get to a function which does fs:18, which you then realize is the PCR,
you then walk back and realize pcr->0x1c is PCRB, and Prcb->0x124 is
current thread.
Yes, this example could easily be destroyed by saying " use a #define
with inline assembly" but I can bring many more; we can't start using
inline assembly everywhere... msvc does an amazing job at optimizing
these things, and even gcc isn't that bad, if only you let it. Code
built without -o2 makes horrible usage of the stack, which makes you
have to memory a lot more addresses then code which simple stores values
in registers. Because humans are smart, the loops generated by -O2 are
also much closer to what someone that understands assembly is used to
(for example, the loop will use ecx, and not a stack variable that you
need to memorize). I consider myself an expert on assembly coding, and I
simply have great trouble reading non-O2 kernels, so how exactly does it
help debugging?
In the end, I am convinced that the only disadvantage of using -O2 by
default is that it will slightly increase build times. I don't think
this increase is more then, at most 1 minute or two for a complete
build. If this issue is really critical to someone people, then perhaps
only core system files should use -O2 (kernel32, ntdll, ntoskrnl, csr,
win32k, drivers, etc).
I know some of the developers on IRC are strongly for this, but I want
to make sure I get a broader opinion.
Best regards,
Alex Ionescu
Ge van Geldorp wrote:
> Yup, using Insight (the cygwin GUI for gdb), which connects
> over a null
> modem to the GDB stub in ntoskrnl. I'm using a
> slightly-modified version,
> since the official one insists that when doing a stack
> backtrace the frame
> pointer should increase for each stack frame (which is
> correct generally but
> breaks when you're backtracing from the kernelmode stack back into the
> usermode stack). Also the official version has a problem with hardware
> watchpoints.
I use the Win32 port which comes as an add-on package with DevCpp.
However I've recently switched my pen drive compiler from DevCpp to
Code::Blocks
I'm finding the Code::Blocks built in debugger (which also uses mingw gdb)
to be excellent and have started using that in place of Insight, as it
doesn't seem have any of the problems Insight has.
In fact, I'm so impressed with Code::Blocks as a whole, I think it makes
DevCpp obsolete and needs a backend writing for rbuild. It's certainly the
best mingw enabled development tool for ROS I've come across. ;)
Ged.
************************************************************************
The information contained in this message or any of its
attachments is confidential and is intended for the exclusive
use of the addressee. The information may also be legally
privileged. The views expressed may not be company policy,
but the personal views of the originator. If you are not the
addressee, any disclosure, reproduction, distribution or other
dissemination or use of this communication is strictly prohibited.
If you have received this message in error, please contact
postmaster(a)exideuk.co.uk
<mailto:postmaster@exideuk.co.uk> and then delete this message.
Exide Technologies is an industrial and transportation battery
producer and recycler with operations in 89 countries.
Further information can be found at www.exide.com
jimtabor(a)svn.reactos.org wrote:
> Clean up and fixed NtUserGetMenuItemRect. Needs more testing. I watch mplayerc swip all the menu items with out crashing.
>
>
> Updated files:
> trunk/reactos/subsys/win32k/ntuser/menu.c
>
*out
Hi,
it seems, some people have problems to install reactos from the boot cd.
The reason is, all newer pc with a P4-HT or an AMD64X2 cpu are
multiprocessor compliant. In this case, the auto choice is
multiprocessor for the computer type. If the choice isn't changed, the
multiprocessor hal and the uniprocessor kernel are installed. The next
boot will show an empty colored (light green or red) screen after
freeldr has started reactos. I think, we should disable the auto choice
for the computer type.
- Hartmut
Thomas Weidenmueller wrote:
> Murphy, Ged (Bolton) wrote:
> > which, IIRC (I don't have a compiler to hand) would throw
> up 'assignment in
> > expression' errors with our MSVC /w4 setting. We would
> therefore need
> > something like :
>
> Then don't use that warning level if it rejects good code that's not
> even confusing.
It's set via rbuilds MSDN backend. It needs functionality to set different
warning levels for different components. e.g. /w4 for core components only.
Having to do it manually is far too much effort for your average programmer
;)
Maybe I should stop moaning and correct it .... hmmm :p
> > Which has suddenly become rather long for a normally simple
> message loop
> > routine ..... LOL
>
> It's exactly the same, just ugly IMO.
I know, as I said, it was just for fun.
First day back at work after the Christmas break is hard going ;)
(and why are these mails being sent to both ros-diff and ros-dev when only
addressed to ros-dev, Gé ?)
Ged.
************************************************************************
The information contained in this message or any of its
attachments is confidential and is intended for the exclusive
use of the addressee. The information may also be legally
privileged. The views expressed may not be company policy,
but the personal views of the originator. If you are not the
addressee, any disclosure, reproduction, distribution or other
dissemination or use of this communication is strictly prohibited.
If you have received this message in error, please contact
postmaster(a)exideuk.co.uk
<mailto:postmaster@exideuk.co.uk> and then delete this message.
Exide Technologies is an industrial and transportation battery
producer and recycler with operations in 89 countries.
Further information can be found at www.exide.com
Hi GreatLord!
greatlrd(a)svn.reactos.org wrote:
> forget remove a calc value in the for loop it can do outside the loop.
>
>
>
> Updated files:
> trunk/reactos/subsys/win32k/dib/dib16bpp.c
>
I'm getting this even after a make clean,
[CC] subsys/win32k/dib/dib16bpp.c
subsys/win32k/dib/dib16bpp.c: In function `CheckClipRegion':
subsys/win32k/dib/dib16bpp.c:631: error: structure has no member named `Height'
make[1]: *** [obj-i386/subsys/win32k/dib/dib16bpp.o] Error 1
Thanks,
James