code cleanups: 64-bit sids and improved sys_millis

[Ostap Cherkashin]

Even though bandicoot is already using "long long" (C99) for 64-bit integer types for the relational language, some important numbers in the system are still 32- or 64-bits long (depending on the architecture). One of these numbers is the time returned by sys_millis(), though it is used for test code only (so far). Another number you can frequently see in the logs:

---
[E] 2011-08-03 19:17:22 UTC, 0000000000000060 method P, path /store_summary_res_2, time 12ms - 200
---

That long number is a "version number" or "transaction number" and internally it is called sid (statement identifier). Each function invocation (even the one which is read-only) results in the increment of this number and it is extensively used in the transaction manager to figure out which versions of variables to read or write (there is more on this subject on our blog).

The attached patch makes sid and the result of sys_millis() "long long" (or at least 64-bit in C terms). This also ends up in a complete avoidance of #ifdefs. Any comments or suggestions?

- Ostap

[Michel Martens]

On Wed, Aug 3, 2011 at 4:59 PM, Ostap Cherkashin <os...@bandilab.org> wrote:
> Any comments or suggestions?
>

Can you put this in a branch? I have no problems patching the version
I'm using, but maybe there can be an experimental branch that we can
follow closely.

[Ostap Cherkashin]

I created the "origin/long_long" branch for this particular patch and pushed it to git://bandilab.org/bandicoot. This way I can update it with the review comments independently of the primitive parameters work (which should probably as well go into a separate branch). Eventually these patches should end up in the master, and at that point they will get merged all together.

[Michel Martens]

On Wed, Aug 3, 2011 at 8:01 PM, Ostap Cherkashin
<ostap.ch...@gmail.com> wrote:
> I created the "origin/long_long" branch for this particular patch and pushed it to git://bandilab.org/bandicoot. This way I can update it with the review comments independently of the primitive parameters work (which should probably as well go into a separate branch). Eventually these patches should end up in the master, and at that point they will get merged all together.

Excellent, thanks a lot :-)

[julo]

I've had a look at the patch and here are my comments:

1) there are changes in usage of sizeof() in sys_write calls. This
makes sense but there are still one place in tuple.c where we use
sizeof(int) instead of sizeof(variable). What about using the same
approach for the other sizeof() use cases, for example in mem_alloc
calls?

2) there are use cases of sid and sys_millis() in test/perf/*.c files
and are not changed in the patch.

3) there are several places in the code where we convert sid to a
string using %016llX pattern. It would be better to have this coded in
one place only (str_from_sid method may be?). We could also have
another sys_log method with additional, explicit, sid parameter and
hide the conversion.


The last point is not a result of this patch but it might be worth
addressing now.

>  0001-64bit-sids-and-improved-sys_millis.patch
> 54KViewDownload

[Ostap Cherkashin]

Thanks for the review. I have fixed #1 and #2 and I also merged it with the updated master. With regard to #3, not all of the sys_log use cases print SIDs, so maintaining two different versions of this procedure (yet still accepting the standard printf(3) flags) does not seem to be of a great benefit to me. I would leave it as is.

[Julius Chrobak]

hi Ostap. I've reviewed the latest changes and I've also run a test on
my 32-bit Windows. Everything is working fine. I have no more comments
for this branch.

On Aug 8, 11:14 pm, Ostap Cherkashin <ostap.cherkas...@gmail.com>
wrote:

[Ostap Cherkashin]

I merged it into the master and removed the branch.