On 11/13/2018 4:08 PM, Jonathan Nieder wrote:
Hi again,
Ben Peart wrote:
On 11/13/2018 1:18 PM, Jonathan Nieder wrote:
Ben Peart wrote:
Why introduce a new setting to disable writing the IEOT extension instead of
just using the existing index.threads setting? If index.threads=1 then the
IEOT extension isn't written which (I believe) will accomplish the same
goal.
Do you mean defaulting to index.threads=1? I don't think that would
be a good default, but if you have a different change in mind then I'd
be happy to hear it.
Or do you mean that if the user has explicitly specified index.threads=true,
then that should imply index.recordOffsetTable=true so users only have
to set one setting to turn it on? I can imagine that working well.
Reading the index with multiple threads requires the EOIE and IEOT
extensions to exist in the index. If either extension doesn't exist, then
the code falls back to the single threaded path. That means you can't have
both 1) no warning for old versions of git and 2) multi-threaded reading for
new versions of git.
If you set index.threads=1, that will prevent the IEOT extension from being
written and there will be no "ignoring IEOT extension" warning in older
versions of git.
With this patch 'as is' you would have to set both index.threads=true and
index.recordOffsetTable=true to get multi-threaded index reads. If either
is set to false, it will silently drop back to single threaded reads.
Sorry, I'm still not understanding what you're proposing. What would be
- the default behavior
- the mechanism for changing that behavior
under your proposal?
I consider index.threads=1 to be a bad default. I would understand if
you are saying that that should be the default, and I tried to propose
a different way to achieve what you're looking for in the quoted reply
above (but I'm not understanding whether you like that proposal or
not).
Today, both the write logic (ie should we write out the IEOT extension)
and the read logic (should I use the IEOT, if available, and do
multi-threaded reading) are controlled by the single "index.threads"
setting. I would like to keep the settings as simple as possible to
prevent user confusion.
If we have two different settings (index.threads and
index.recordoffsettable) then the only combination that will result in
the user actually getting multi-threaded reads is when they are both set
to true. Any other combination will silently fail. I think it would be
confusing if you set index.threads=true and got no error message but
didn't get multi-threaded reads either (or vice versa).
If you want to prevent any of the scary "ignoring IEOT extension" from
ever happening then your only option is to turn off the IEOT writing by
default. The downside is that people have to discover and turn it on if
they want the improvements. This can be achieved by changing the
default for index.threads from "true" to "false."
diff --git a/config.c b/config.c
index 2ee29f6f86..86f5c14294 100644
--- a/config.c
+++ b/config.c
@@ -2291,7 +2291,7 @@ int git_config_get_fsmonitor(void)
int git_config_get_index_threads(void)
{
- int is_bool, val = 0;
+ int is_bool, val = 1;
val = git_env_ulong("GIT_TEST_INDEX_THREADS", 0);
if (val)
If you want to provide a way for a concerned user to disable the message
after the first time they have seen it, then they can be instructed to
run 'git config --global index.threads false'
There is no way to get multi-threaded reads and NOT get the scary
message with older versions of git. Multi-threaded reads require the
IEOT extension to be written into the index and the existence of the
IEOT extension in the index will always generate the scary warning.
Jonathan
