{"thread":{"id":"36071","subject":"RFC GSoC idea: git configuration caching (needs co-mentor!)","startedAt":"2014-03-06T05:57:20Z","lastAt":"2014-03-06T21:33:14Z","messageCount":4,"participants":["Michael Haggerty","Junio C Hamano","Jeff King"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"236148","messageId":"53180E40.5050308@alum.mit.edu","threadId":"36071","inReplyTo":null,"subject":"RFC GSoC idea: git configuration caching (needs co-mentor!)","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2014-03-06T05:57:20Z","receivedAt":"2014-03-06T05:57:20Z","isPatch":false,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"I just wrote up the idea that fell out of the discussion [1] about the\nother configuration features that I proposed.  As far as I am concerned,\nit can be merged as soon as somebody volunteers as a co-mentor.  The\nidea is embodied in a pull request against the git.github.io repository\n[2]; the text is also appended below for your convenience.\n\nMichael\n\n[1] http://article.gmane.org/gmane.comp.version-control.git/242952\n[2] https://github.com/git/git.github.io/pull/7\n\n### git configuration API improvements\n\nThere are many places in Git that need to read a configuration value.\nCurrently, each such site calls `git_config()`, which reads and parses\nthe configuration files every time that it is called.  This is\nwasteful, because it results in the configuration files being\nprocessed multiple times during a single `git` invocation.  It also\nprevents the implementation of potential new features, like adding\nsyntax to allow a configuration file to unset a previously-set value.\n\nThis goal of this project is to make configuration work as follows:\n\n* Read the configuration from files once and cache the results in an\n  appropriate data structure in memory.\n\n* Change `git_config()` to iterate through the pre-read values in\n  memory rather than re-reading the configuration files.\n\n* Add new API calls that allow the cache to be inquired easily and\n  efficiently.  Rewrite other functions like `git_config_int()` to be\n  cache-aware.\n\n* Rewrite callers to use the new API wherever possible.\n\nYou will need to consider how to handle other config API entry points\nlike `git_config_early()` and `git_config_from_file()`, as well as how\nto invalidate the cache correctly in the case that the configuration\nis changed while `git` is executing.\n\nSee\n[this mailing list\nthread](http://article.gmane.org/gmane.comp.version-control.git/242952)\nfor some discussion about this and related ideas.\n\n - Language: C\n - Difficulty: medium\n - Possible mentors: Michael Haggerty\n\n-- \nMichael Haggerty\nmhagger@alum.mit.edu\nhttp://softwareswirl.blogspot.com/\n"},{"id":"236183","messageId":"xmqqtxbbxh99.fsf@gitster.dls.corp.google.com","threadId":"36071","inReplyTo":"53180E40.5050308@alum.mit.edu","subject":"Re: RFC GSoC idea: git configuration caching (needs co-mentor!)","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-03-06T19:24:18Z","receivedAt":"2014-03-06T19:24:18Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Michael Haggerty <mhagger@alum.mit.edu> writes:\n\n> I just wrote up the idea that fell out of the discussion [1] about the\n> other configuration features that I proposed.  As far as I am concerned,\n> it can be merged as soon as somebody volunteers as a co-mentor.  The\n> idea is embodied in a pull request against the git.github.io repository\n> [2]; the text is also appended below for your convenience.\n>\n> Michael\n>\n> [1] http://article.gmane.org/gmane.comp.version-control.git/242952\n> [2] https://github.com/git/git.github.io/pull/7\n>\n> ### git configuration API improvements\n>\n> There are many places in Git that need to read a configuration value.\n> Currently, each such site calls `git_config()`, which reads and parses\n> the configuration files every time that it is called.  This is\n> wasteful, because it results in the configuration files being\n> processed multiple times during a single `git` invocation.  It also\n> prevents the implementation of potential new features, like adding\n> syntax to allow a configuration file to unset a previously-set value.\n>\n> This goal of this project is to make configuration work as follows:\n>\n> * Read the configuration from files once and cache the results in an\n>   appropriate data structure in memory.\n>\n> * Change `git_config()` to iterate through the pre-read values in\n>   memory rather than re-reading the configuration files.\n>\n> * Add new API calls that allow the cache to be inquired easily and\n>   efficiently.  Rewrite other functions like `git_config_int()` to be\n>   cache-aware.\n\nAre you sure about the second sentence of this item is what you\nwant?\n\ngit_config_<type>(name, value) are all about parsing \"value\" (string\nor NULL) as <type>, return the parsed value or complain against a\nbad value for \"name\".  They do not care where these \"name\" and\n\"value\" come from right now, and there is no reason for them to\nstart caring about caching.  They will still be the underlying\nhelper functions the git_config() callbacks will depend on even\nafter the second item in your list happens.\n\nA set of new API calls would look more like this, I would think:\n\n\textern int git_get_config_string_multi(const char *, int *, const char ***);\n\tconst char **values;\n        int num_values;\n\n        if (git_get_config_string_multi(\"sample.str\", &num_values, &values))\n        \treturn -1;\n\tprintf(\"[sample]\\n\");\n\tfor (i = 0; i < num_values; i++)\n\t\tprintf(\"  str = %s\\n\", value[i]);\n\tprintf(\"\\n\");\n\tfree(values);\n\nwith a \"singleton\" wrapper that may be in essense:\n\n\tconst char *git_get_config_string(const char *name)\n        {\n\t\tconst char **values, *result;\n                int num_values;\n\n\t        if (git_get_config_string_multi(\"sample.str\", &num_values, &values))\n        \t\treturn NULL;\n                result = num_values ? values[num_values - 1] : NULL;\n                free(values);\n\t\treturn result;\n\t}\n\nthat implements the \"last one wins\" semantics.  The real thing would\nneed to avoid allocation and free overhead.\n\n> * Rewrite callers to use the new API wherever possible.\n>\n> You will need to consider how to handle other config API entry points\n> like `git_config_early()` and `git_config_from_file()`, as well as how\n> to invalidate the cache correctly in the case that the configuration\n> is changed while `git` is executing.\n>\n> See\n> [this mailing list\n> thread](http://article.gmane.org/gmane.comp.version-control.git/242952)\n> for some discussion about this and related ideas.\n>\n>  - Language: C\n>  - Difficulty: medium\n>  - Possible mentors: Michael Haggerty\n"},{"id":"236189","messageId":"20140306194654.GA28203@sigill.intra.peff.net","threadId":"36071","inReplyTo":"xmqqtxbbxh99.fsf@gitster.dls.corp.google.com","subject":"Re: RFC GSoC idea: git configuration caching (needs co-mentor!)","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2014-03-06T19:46:54Z","receivedAt":"2014-03-06T19:46:54Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Mar 06, 2014 at 11:24:18AM -0800, Junio C Hamano wrote:\n\n> > * Add new API calls that allow the cache to be inquired easily and\n> >   efficiently.  Rewrite other functions like `git_config_int()` to be\n> >   cache-aware.\n> \n> Are you sure about the second sentence of this item is what you\n> want?\n> \n> git_config_<type>(name, value) are all about parsing \"value\" (string\n> or NULL) as <type>, return the parsed value or complain against a\n> bad value for \"name\".  They do not care where these \"name\" and\n> \"value\" come from right now, and there is no reason for them to\n> start caring about caching.  They will still be the underlying\n> helper functions the git_config() callbacks will depend on even\n> after the second item in your list happens.\n\nYeah, I agree we want a _new_ set of helpers for retrieving values in a\nnon-callback way. We could call those helpers \"git_config_int\" (and\nrename the existing pure functions), but I'd rather not, as it simply\ninvites confusion with the existing ones.\n\n> A set of new API calls would look more like this, I would think:\n> \n> \textern int git_get_config_string_multi(const char *, int *, const char ***);\n\nNot important at this stage, but I was hoping we could keep the names of\nthe new helpers shorter. :)\n\n> \tconst char *git_get_config_string(const char *name)\n>         {\n> \t\tconst char **values, *result;\n>                 int num_values;\n> \n> \t        if (git_get_config_string_multi(\"sample.str\", &num_values, &values))\n>         \t\treturn NULL;\n>                 result = num_values ? values[num_values - 1] : NULL;\n>                 free(values);\n> \t\treturn result;\n> \t}\n> \n> that implements the \"last one wins\" semantics.  The real thing would\n> need to avoid allocation and free overhead.\n\nOne of the things that needs to be figured out by the student is the\nformat of the internal cache. I had actually envisioned a mapping of\nkeys to values, where values are represented as a full list of strings.\nThen your \"string_multi\" can just return a pointer to that list, and a\nlast-one-wins lookup can grab the final value, with no allocation or\nownership complexity. We'd lose the relative order of different config\nkeys, but those should never be important (only the order of single\nkeys, but that is reflected in the order of the value list).\n\nAnother approach would be to actually represent the syntax tree of the\nconfig file in memory. That would make lookups of individual keys more\nexpensive, but would enable other manipulation. E.g., if your syntax\ntree included nodes for comments and other non-semantic constructs, then\nwe can use it for a complete rewrite. And \"git config\" becomes:\n\n  1. Read the tree.\n\n  2. Perform operations on the tree (add nodes, delete nodes, etc).\n\n  3. Write out the tree.\n\nand things like \"remove the section header when the last item in the\nsection is removed\" become trivial during step 2.\n\nBut comparing those approaches is something for the student to figure\nout, I think.\n\n-Peff\n"},{"id":"236206","messageId":"5318E99A.20708@alum.mit.edu","threadId":"36071","inReplyTo":"xmqqtxbbxh99.fsf@gitster.dls.corp.google.com","subject":"Re: RFC GSoC idea: git configuration caching (needs co-mentor!)","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2014-03-06T21:33:14Z","receivedAt":"2014-03-06T21:33:14Z","isPatch":false,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"On 03/06/2014 08:24 PM, Junio C Hamano wrote:\n> Michael Haggerty <mhagger@alum.mit.edu> writes:\n> \n>> I just wrote up the idea that fell out of the discussion [1] about the\n>> other configuration features that I proposed.  As far as I am concerned,\n>> it can be merged as soon as somebody volunteers as a co-mentor.  The\n>> idea is embodied in a pull request against the git.github.io repository\n>> [2]; the text is also appended below for your convenience.\n>>\n>> Michael\n>>\n>> [1] http://article.gmane.org/gmane.comp.version-control.git/242952\n>> [2] https://github.com/git/git.github.io/pull/7\n>>\n>> ### git configuration API improvements\n>>\n>> There are many places in Git that need to read a configuration value.\n>> Currently, each such site calls `git_config()`, which reads and parses\n>> the configuration files every time that it is called.  This is\n>> wasteful, because it results in the configuration files being\n>> processed multiple times during a single `git` invocation.  It also\n>> prevents the implementation of potential new features, like adding\n>> syntax to allow a configuration file to unset a previously-set value.\n>>\n>> This goal of this project is to make configuration work as follows:\n>>\n>> * Read the configuration from files once and cache the results in an\n>>   appropriate data structure in memory.\n>>\n>> * Change `git_config()` to iterate through the pre-read values in\n>>   memory rather than re-reading the configuration files.\n>>\n>> * Add new API calls that allow the cache to be inquired easily and\n>>   efficiently.  Rewrite other functions like `git_config_int()` to be\n>>   cache-aware.\n> \n> Are you sure about the second sentence of this item is what you\n> want?\n> \n> git_config_<type>(name, value) are all about parsing \"value\" (string\n> or NULL) as <type>, return the parsed value or complain against a\n> bad value for \"name\".  They do not care where these \"name\" and\n> \"value\" come from right now, and there is no reason for them to\n> start caring about caching.  They will still be the underlying\n> helper functions the git_config() callbacks will depend on even\n> after the second item in your list happens.\n\nYou're right of course.  For some reason I had it in my brain that these\nfunctions retrieved *and* parsed values, as opposed to just parsing them.\n\nI just fixed the text and pushed it live.\n\nMichael\n\n-- \nMichael Haggerty\nmhagger@alum.mit.edu\nhttp://softwareswirl.blogspot.com/\n"}]}