patchgperf - Patches: patch #9545, Add option to make -k be a list of...

 
 

You are not allowed to post comments on this tracker with your current authentication level.

patch #9545: Add option to make -k be a list of key position restrictions instead of a specification

Submitter:  Frank Wojcik <fwojcik>
Submitted:  Sun 21 Jan 2018 05:22:18 AM UTC
   
 
Category:  None Priority:  5 - Normal
Status:  None Privacy:  Public
Assigned to:  None Open/Closed:  Open

Mon 17 Sep 2018 01:38:03 AM UTC, comment #4: 


> I suspect it will be easier to explain a different use of the existing syntax, and it will be more flexible.


"More flexible" with respect to hypothetical use-cases is bloat and a waste of coding effect.

As long as you have one use-case, and other use-case are hard to imagine, it's better to support just that use-case, with options that are as simple as possible to understand.

(Call me short-sighted, if you want. But that's my experience.)

Bruno Haible <haible>
Group administrator
Sun 16 Sep 2018 10:41:07 PM UTC, comment #3: 

Thanks for the feedback! I will recode the patch to take a new, separate specification.

It could be that a simple "positions <= N" path will suffice for this use case (I will have to double-check on that), and if it does then I will probably code up and submit an alternate patch implementing that solution.

However, I suspect it will be easier to explain a different use of the existing syntax, and it will be more flexible. It may also assist in the work in bug #53114. I will see how things work out.

Frank Wojcik <fwojcik>
Sat 08 Sep 2018 01:15:48 PM UTC, comment #2: 

I don't much like an option (-A) that influences the way another option (-k) is interpreted. Such a behaviour is hard to explain and easy to misuse. I find it better if all options have a clearly defined meaning on their own.

This would mean, the -A option should take a list of positions as argument. Or, can we simplify that without killing the use-case? '-A n' would tell gperf to allow only positions <= n, and disallow the '$' position.

Bruno Haible <haible>
Group administrator
Sat 10 Feb 2018 01:06:50 AM UTC, comment #1: 

Adding rev2 of this patch. This fixes 2 bugs in the previous one:

1) LASTCHAR was not being used as a default allowed position, because set_useall() doesn't set it, and
2) 'make check' was failing because of the added command-line option.

(file #43226)

Frank Wojcik <fwojcik>
Sun 21 Jan 2018 05:22:18 AM UTC, original submission:  

One of my primary use cases for gperf is to test strings against a membership list consisting of roughly 100-200 strings, all of which can be quite long (>100 characters). gperf can easily create hash functions that access input characters far beyond position 1. While correct and functional, this has caused some cache-related issues, and I sought a way to improve it.

At first, I thought the -k option could fix the problem. The documentation led me to believe that I could restrict the possible key positions. However, specifying (e.g.) -k1-15 made gperf use all 15 characters, which is not what I was hoping for.

The attached patch does 2 things:
1) Make a slight change to the -k documentation which makes it clear that the positions specified using it are used exactly, and
2) Add a new flag which causes gperf to treat the positions from -k as a restriction instead of an exact specification.

I'm totally open to a different option name and/or letter, and will happily respin the patch.

I didn't see any tests of -k, so this diff doesn't add any testing. Let me know if something should be added there; ideally with a sketch of what that testing might cover.

Frank Wojcik <fwojcik>

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attached Files
file #43226:  keypos2.diff added by fwojcik (8KiB - text/x-patch)
file #42996:  gperf-1.diff added by fwojcik (8KiB - text/x-patch)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by haible (Posted a comment)
  • -email is unavailable- added by fwojcik (Submitted the item)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

     

    Follow 2 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2018-02-10 fwojcik Attached File- Added keypos2.diff, #43226
    2018-01-21 fwojcik Attached File- Added gperf-1.diff, #42996

    Back to the top

    Powered by Savane 3.12.
    Corresponding source code