Hi Mark,
please accept my congratulations with signing SCA and excuses for not
responding for such a long time. Your contribution looks like a great
feature and I believe it'll be really useful in some scenarios.
I was assigned to review this patch and expect it'll take a few
review cycles until we will end up with a "good to push" patch.
Ok, now a few issues that were identified during quick overview of the
patch...
First of all we need to put your patch in conformance with MySQL
Internals Coding Guidelines, as described at:
http://forge.mysql.com/wiki/MySQL_Internals_Coding_Guidelines
The top three rules are:
- for identation use two spaces (tabs are disallowed);
- open braces should be on a separate line (except for "switch");
- for assigning values to variables we use var= val (this makes easier
to search for assignments using grep tool);
The second point is that for most system calls we use mysys wrappers.
These wrappers mainly guarantee portability. In our case we should use
these wrappers for mapping/unmapping index file. See also mysys/my_mmap.c.
We need to extend the patch, so this feature can be easily
disabled/enabled. I'd suggest to add myisam_use_mmap_for_index variable
for this purpose (for an example see how myisam_use_mmap is implemented).
That's all for now. Please let me know if you have any
concerns/questions.
Thanks,
Sergey
--
Sergey Vojtovich <svoj@stripped>
MySQL AB, Software Engineer
Izhevsk, Russia, www.mysql.com
