Re: mmap patch

I'm just going to play open cards in my reply. No more contests, no more
"look how good I am". Just my true intentions, on a plate. I promised
Harish I was going to change that attitude. So I will.

I will also technically comment where necessary.

On Wed, 2006-07-19 at 16:29 -0400, Jeffrey Stedfast wrote:

> Well, I figured out why you're getting alignment problems... you're
> using int* pointers to decode stuff. You need to use a char*

- technical -

There's no data alignment problems in the latest patch. 

http://pvanhoof.be/files/evolution_data_server__mmap_summary.diff


> Secondly, your code is just really gross:
> 
> - Why do you have a filepos pointer? it's completely unnecessary.

- technical -

Because it's a hack. Initially I didn't want to change the internal API,
so I had no other options but to put it in the CamelFolderSummary
struct.

After federico's three points I , however, changed the internal API by
removing the FILE* from the methods that aren't using it anymore. A
better way would have been to pass the mmap addr position here, in stead
of the FILE*.

Note that the current patch isn't intended for HEAD. It is (for me)
mainly intended for testing purposes. 

If upstream is really interested, they will probably change the summary
file-format drastically anyway. You where the first to say that for mmap
it would be better to change that file format. I was the first to
acknowledge that. I guess we are in agreement here.


- open cards -

In fact, Jeff, if you want to know why I really really started the work:

I did it because I wanted to get things moving in Camel-land. Because
people, like maybe you, would be awaken and/or become passionate about
wanting to do the disk-summary branch so badly ... that they will just
start making it really happen.

It might surprise you, but in a way I was also happy that you said you
questioned this patch by mentioning the disk-summary branch. I hope I
inspired you to go for it.

In another way I was of course a little bit pissed. But it's passion
that moves things. Anger that makes passion. You fueled me to implement
the three points federico asked for. Without your fuel, I wouldn't have
done that.

I really believe in the concepts you and NotZed outlined with the disk
summary idea. Even a little bit more than I believe in my own mmap
ideas. The combination however ... a combination would be awesome.

I really think that a "just go for it"-team can make this happen. I
really believe that you, Jeffrey, could steer such a team.

I might often present myself as an asshole. My intention is to get
things moving. I want us to make things better. A lot better. Just
because we can. For me, that's definitely a reason why we should.

Evolution doesn't have to bleed to death (which is btw what a lot people
predict to happen). There's indeed a lot good stuff in it. Good stuff
that you guys made happen. I acknowledge that, if that is what you
wanted to hear from me. I too can read code myself and figure out who
the authors are. I too respect their work. I also respect the new guys.

> - Why do you use a ptr32 thing? That's gross. There are far cleaner ways
> of doing this... it's also the source of your alignment problems.

- technical -

All the "ptr32" variables are removed in the latest patch, they've been
replaced with the get_unaligned_u32 macro. Are you sure you checked the
latest patch?

If I grep for "ptr32" in the latest patch, there's no occurrences.

> - Why do you have ::needs_free and ::uid_needs_free? Please find another
> way to do this. This is a horrible hack.
> 
> Attached you'll find the beginnings of a much cleaner implementation of
> the mmap approach.

- open cards -

That's really great, you cleaning this up means you are considering the
patch. That is my intention.

It's good to know that you are at least looking at it.


- technical -

What matters for me (and also for tinymail) is that the data isn't
allocated by malloc if it doesn't need to be allocated. If it can be
read from by mmap, it's better for mobile devices to read it that way.

>From the beginning I was very uncertain that the patch would be fit for
daily Evolution usage. I turned out to just work.

A reason for the ugly pieces of that patch, is that uncertainty. 

- open cards -

Initially this was, indeed, a tryout. An experiment. A hack. I openly
admit that I had no idea if it would work with Evolution.

I was concerned people would look at me as somebody who's only intention
was to fork Camel for tinymail. Therefore I added four days of hacking
to the patch to make it work on Evolution.

I didn't have to do that, because after the second day .. it worked with
tinymail already. You can follow the dates in my blog. If I wanted to be
that forking asshole, I would have done a camel-lite (oh wait, I did).

Anyway .. the point that I'm trying to make is: I wouldn't have tried to
make it work with Evolution. Nobody would have cared a lot, and I
wouldn't have to write this open-cards E-mail.

Finally ... who (re)implements the hack is not really what I care about
most. I would be very interested to, in team, with you (or other people)
"just go for it" and (re)implement it in a clean and perfect way. I
would be equally interested to go for your disk summary ideas. Heck, I
would even join your team if their mission was to replace Camel with
your libspruce in Evolution. And I do know how insane that would be.



ps. What I expressed here are my true intentions. For me, there's
nothing more pure than true intentions. Not even life itself.

-- 
Philip Van Hoof, software developer at x-tend 
home: me at pvanhoof dot be 
gnome: pvanhoof at gnome dot org 
work: vanhoof at x-tend dot be 
http://www.pvanhoof.be - http://www.x-tend.be