Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

std.uni - speed up Trie construction by factor of ~3 #1969

Merged
merged 1 commit into from
Feb 28, 2014

Conversation

DmitryOlshansky
Copy link
Member

Take advantage of word-at-once checking for slices of bit-packed arrays in the trie.

Now it's seems to be either on par or much faster (depending on the set) then the venerable std.regex internal trie.

Tiny test:

import std.uni;

void main()
{
    import std.regex, std.uni, std.stdio;
    import std.datetime, core.memory;
    import std.internal.uni_tab, std.internal.uni;
    StopWatch sw;
    GC.disable();
    sw.start();
    foreach(_; 0..1_000)
        auto set = unicode.Nd;  
    sw.stop();
    writefln("Find set - %s us", sw.peek().usecs);

    auto set = unicode.Nd;
    sw.reset();
    sw.start();
    foreach(_; 0..1_000)
        auto ct1 = set.codepointSetTrie!(10, 11);
    sw.stop();
    writefln("Build Trie - %s us", sw.peek().usecs);

    sw.reset();
    sw.start();
    foreach(_; 0..1_000)
        auto ct2 = std.internal.uni.CodepointTrie!8(unicodeNd);
    sw.stop();
    writefln("Build Trie old - %s us", sw.peek().usecs);
}

@@ -1222,11 +1222,12 @@ private:
// this construct doesn't own memory, only provides access, see MultiArray for usage
@trusted struct PackedArrayViewImpl(T, size_t bits)
{
pure nothrow:
pure :
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happened? Who's throwing?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops. debug writeln must have been throwing I'll get nothrow back.

Take advantage of word-at-once checking for slices of bit-packed
arrays in the trie.
@DmitryOlshansky
Copy link
Member Author

@monarchdodra Got back that nothrow

@monarchdodra
Copy link
Collaborator

I don't see anything wrong in the pull, but I can't really claim to understand what you are doing either. I'm really just merging your pulls based off of trust alone. That's mostly fine by me, but do you know anyone who could actually review it?

@DmitryOlshansky
Copy link
Member Author

That's mostly fine by me, but do you know anyone who could actually review it?

@monarchdodra not really and it kinda sucks.
Ideally (Static)Trie construction will go into std.container one day and be reviewed there to bloody death. It was part of the plan until I understood that:

a) Good public interface is hard to expose
b) Was not generic enough contrary to my belief
c) It introduces the whole concept of bit-packing arbitrary integers, not just bools. That is it contains something more general then BitArray. Got to make it public then - a review in its own right.
d) std.container is neglected and there is no vision shared on its fate, presumpby a significant redesign/tweak. I decided to not add junk there that may need to move anyway.

And postponed that indefinitely.
All in all, the last rounds of tweaks around std.uni are things that I need to sort out before killing off the old std.internal.uni and freeing up some 100Kb of object code out of Phobos :)

@monarchdodra
Copy link
Collaborator

Auto-merge toggled on

@braddr
Copy link
Member

braddr commented Feb 27, 2014

Pull updated, auto_merge toggled off

@DmitryOlshansky
Copy link
Member Author

Meh update dropped, I swear I didn't update it :)

@yebblies
Copy link
Member

Auto-merge toggled on

yebblies added a commit that referenced this pull request Feb 28, 2014
std.uni - speed up Trie construction by factor of ~3
@yebblies yebblies merged commit 9e2c3f1 into dlang:master Feb 28, 2014
@DmitryOlshansky
Copy link
Member Author

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants