Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP
branch: master
Commits on Dec 7, 2011
  1. @markw65 @scottmac

    [Fix] ArrayIterator::valid was incorrect

    markw65 authored scottmac committed
    Summary:
    ArrayIterator::valid was implemented as current() !== false, which would
    incorrectly terminate when an element's value was false. Test key() !== null
    instead (noting that keys cant be null).
    
    Test Plan:
    fast_tests, flib/type/dict
    
    This is just a revert of a bogus change. Adding a new test case to prevent a
    repeat.
    
    <?php
    
    function test($a) {
      $it = new ArrayIterator($a);
      while ($it->valid()) {
        var_dump($it->key());
        var_dump($it->current());
        $it->next();
      }
      }
    
    test(array('a' => 'x',
               false => 'y',
               '1' => false,
               null => 'z',
               'c' => 'w'));
    
    DiffCamp Revision: 144935
    Reviewed By: hzhao
    CC: hphp-diffs@lists, hzhao
    Revert Plan:
    OK
  2. @scottmac

    Fix buffer overflow in glob()

    scottmac authored
    Summary:
    glob() needs the directory to be less than PATH_MAX
    
    Test Plan:
    make
    make fast_tests
    
    DiffCamp Revision: 162960
    Reviewed By: hzhao
    CC: hzhao, hphp-diffs@lists
    Revert Plan:
    Ok
  3. @markw65 @scottmac

    Dont include idx in the hphpi separable extensions

    markw65 authored scottmac committed
    Summary: idx is too specific to facebook, and isnt compatible with phabricator
    
    Differential Revision: 281113
  4. @scottmac

    appendstring() expects the 9th parameter to be length, not precision

    scottmac authored
    Summary:
    Fix from upstream, the precision parameter is used for fp conversion
    not for when INF or NaN is being printed. The parameter should be length.
    
    Test Plan:
    make
    make fast_tests
    make slow_tests
    
    DiffCamp Revision: 162973
    Reviewed By: hzhao
    CC: hzhao, hphp-diffs@lists
    Revert Plan:
    Ok
Commits on Nov 29, 2011
  1. @scottmac

    [Fix/apc] typo in profiling

    qigao authored scottmac committed
    Summary:
    A typo makes it not using normalized keys in profiling deletion,
    resulting in incorrect group stats. Does not affect production
    correctness.
    
    Test Plan: make fast_tests
    
    Reviewers: mwilliams, myang
    
    Reviewed By: myang
    
    CC: ps, mwilliams, myang
    
    Differential Revision: 353265
  2. @scottmac

    [APC] Improve apc size profiling

    qigao authored scottmac committed
    Summary:
    This diff does a few things to improve apc size profiling:
    1. More compact format for script to process the result. I changed it to
    one entry a line and got rid of the global object in json.
    2. Add a profiling to monitor the size of keys with noTTL in a group
    3. Switch to use concurrent_hash_map to protect the group and detail
    profiling
    4. change some field from int64 to int32 to save some memory
    
    Test Plan:
    make fast_tests
    production testing on profiling
    Note that all the code touched in this diff is not on critical path if
    profiling is turned off. This is staging for enabling profiling on h2g
    machines.
    
    Reviewers: myang, mwilliams, ps
    
    Reviewed By: mwilliams
    
    CC: ps, mwilliams
    
    Differential Revision: 352855
  3. @markw65 @scottmac

    [Perf] Reduce o_set overhead

    markw65 authored scottmac committed
    Summary:
    ObjectData's o_set family of functions all took a bool flag "forInit". The flag
    was used to bypass access checks, and was only used when initializing an
    ObjectData from hphpi, and from o_setArray. This meant that we were always
    passing in, and checking a mostly unused parameter. In addition, Object's o_set
    family didnt have the flag. code-gen didnt differentiate the two cases, and so
    in some cases was passing the class name as the "forInit" flag. Bizarrely, this
    mostly worked, because the class-name would be converted to true, and access
    would be allowed (which is usually the right thing to do in correct programs).
    
    With this diff, ObjectData's o_set family matches the Object o_set family,
    solving the correctness problem, and also reducing the call overhead by dropping
    a parameter.
    
    Test Plan: fast_tests slow_tests
    
    Reviewers: qigao, myang
    
    Reviewed By: myang
    
    CC: ps, mwilliams, myang, qigao
    
    Differential Revision: 346544
  4. @markw65 @scottmac

    [Fix] Misc reference handling buglets exposed by enabling inlining

    markw65 authored scottmac committed
    Summary:
     - in f($x, $x &= $y), where f takes its first param by reference, we would
       pass a reference to $x after the assignment, rather than before it (this
       happened independent of inlining).
     - in f(g(x)) if g returned by reference, and f did not take its param by
       reference, f could end up with a reference to the result of g anyway, due to
       copy constructor elision (again, independent of inlining)
     - there was a similar problem when a return-by-reference function got inlined.
       The return value could be destroyed before it was used (resulting in a crash in
       one of the added test cases), or it could just end up refering to the wrong
       thing.
     - in $a = &f(), if f returned by value, and got inlined, $a could end up bound
       to the result of f() (rather than bound to a copy of the result of f())
    
    In order to expose the return-by-reference issue, I had to increase the
    inliner's agressiveness. So Ive changed it from a bool, to an int and used it as
    the max "cost" for a function.
    
    To minimize extra temps, I added the return value's refness to the FunctionInfo.
    
    Test Plan: fast_tests, slow_tests and slow_tests with AutoInline=5 (there are
    still some failures, with the latter - will address separately).
    
    Reviewers: qigao, myang
    
    Reviewed By: myang
    
    CC: ps, mwilliams, myang
    
    Differential Revision: 348039
    
    Task ID: 630786
  5. @scottmac

    [Fix/hphpd] Only escape the cases needed

    qigao authored scottmac committed
    Summary:
    This diff makes it hphpd command line tokenizer only escape the cases
    that are needed so that it doesn't do any inappropriate escaping.
    
    Test Plan:
    make fast_tests
    the case in the task
    
    Reviewers: mwilliams
    
    Reviewed By: mwilliams
    
    CC: ps, mwilliams, qigao
    
    Differential Revision: 351968
    
    Task ID: 778864
  6. @scottmac

    [Fix] Fix bug in chunked post data

    myang authored scottmac committed
    Summary:
    The runtime option AlwaysPopulateRawPostData was misused to discard the post
    data rather than only to decide whether to populate $HTTP_RAW_POST_DATA. The
    purpose of discarding post data was an memory optimization on video data
    because the video data is alreay streamed into a temp file so keep the extra
    video data as post data in its entirety isn't needed. However, post data is
    generally needed for non-video data. I fixed that by removing the
    AlwaysPopulateRawPostData check in read_all_post_data.
    
    Test Plan:
    make fast_tests
    <?php
    echo strlen($_POST['foo']);
    Use html form to post data:
    <form action="/temp.php" method="post">
    <input type="hidden" name="foo" value="large data"  /> <input type="submit" />
    </form>
    Before the fix, the returned value is about 4000 due to truncated post data.
    Observed correct value after the fix.
    
    Reviewers: mwilliams, qigao
    
    Reviewed By: mwilliams
    
    CC: hphp-diffs@lists, ps, mwilliams, myang
    
    Differential Revision: 352127
  7. @markw65 @scottmac

    [Fix] Guard AnalysisResult::m_files with a lock

    markw65 authored scottmac committed
    Summary: For parse-on-demand, we were checking to see if a file had already
    been parsed without aquiring a lock. That could result in a crash if files were
    inserted during the lookup.
    
    Test Plan: Running the command from the task resulted in a crash once in about
    10 tries before the fix. After the fix I ran it 3000 times with no failures.
    
    Reviewers: qigao, myang
    
    Reviewed By: qigao
    
    CC: ps, mwilliams, qigao
    
    Differential Revision: 351734
    
    Task ID: 778331
  8. @scottmac

    Mask out xhprof for memory allocation tracking

    ps authored scottmac committed
    Summary:
    When xhprof is enabled, it will allocate quite a bit of memory
    and this can throw off normal memory allocation statistics.  Mask these
    allocations since it is not relevant for measurements.
    
    Test Plan:
    xhprof_enable() followed by memory_get_allocation() seeing no
    increase in allocation memory.
    
    Reviewers: je
    
    Reviewed By: je
    
    CC: jasont, ps, mwilliams, je
    
    Differential Revision: 349035
  9. @scottmac

    [HPHPi] Use ParserBase::IsClosureName to check closure name

    myang authored scottmac committed
    Summary:
    Two places in HPHPi are changed to use ParserBase::IsClosureName to
    check closure name to avoid ad-hoc code.
    
    Test Plan: make fast_tests
    
    Reviewers: mwilliams, qigao
    
    Reviewed By: mwilliams
    
    CC: hphp-diffs@lists, ps, mwilliams
    
    Differential Revision: 351791
  10. @scottmac

    [Fix/hphpd] remove the trailing ';' for '=' command

    qigao authored scottmac committed
    Summary:
    In a recent change, () were added to wrap the expression for '=', which
    made ';' in the expression a syntax error. We were thinking on a general
    fix but it seems hard to do without calling php parser for the command
    line. This diff fixes one common benign case with traling ';', which used
    to work before the () were added.
    
    Test Plan:
    make fast_tests
    tried with hphpd on command line
    hphpd> =123;
    123
    
    Reviewers: mwilliams, myang
    
    Reviewed By: myang
    
    CC: ps, mwilliams, myang, qigao
    
    Differential Revision: 351726
    
    Task ID: 747161
  11. @scottmac

    [Fix] Do not allocate user function id for certain names

    myang authored scottmac committed
    Summary:
    I made a change not to allocate user function id for names resulted from
    create_function and closures. I also removed the warning message, as long
    as we have a substantial user function names have integer ids, there will
    be some benefit even though some "late comers" will have to go though a
    lookup. The warning message was causing some tests to fail.
    
    Test Plan:
    make fast_tests
    load www sandbox
    run server mode flib test
    
    Reviewers: mwilliams, qigao
    
    Reviewed By: qigao
    
    CC: hphp-diffs@lists, ps, mwilliams, qigao, myang
    
    Differential Revision: 351742
  12. @ottoni @scottmac

    Add new TestTraits to TestCodeRun and fix hphpc accordingly

    ottoni authored scottmac committed
    Summary:
    I've recently learned a few more nuances about traits.  I added some
    new tests to TestCodeRun, and fixed the following cases:
    
    1. abstract trait methods may be provided by a base class of the class
       using the trait
    
    2. abstract trait methods may be provided by other traits used in the
       same class
    
    3. trait methods and static properties can be accessed via
       trait_name::blah.
    
    Item 3 abore required enabling code generation for trait classes.
    
    Test Plan:
    make fast_tests
    make slow_tests
    built www and checked output CPP code against previous
    
    Reviewers: mwilliams, myang, qigao
    
    Reviewed By: mwilliams
    
    CC: andrewparoski, ps, mwilliams
    
    Differential Revision: 350654
    
    Task ID: 758990
  13. @scottmac

    [Fix/hphpd] Fix bugs in server api

    qigao authored scottmac committed
    Summary:
    This diff fixes the issue of interruping for the second time causing the
    server to abort. It also makes the state machine more clear and guards
    mroe strict so that user retries are unlikely to cause issues and leave
    server in an error state.
    
    Note: there are some generated files change that are not because of this
    diff.
    
    Test Plan:
    make fast_tests
    manually tried more functionalities
    
    Reviewers: myang
    
    Reviewed By: myang
    
    CC: tuomaspelkonen, ps, mwilliams, myang
    
    Differential Revision: 350417
    
    Task ID: 753544
Commits on Nov 28, 2011
  1. @scottmac

    [Fix/HPHPi] Increase maximum function id

    myang authored scottmac committed
    Summary:
    To suppress the warning as the number of functions can exceed the current
    limit: HipHop Warning: Maximum function id reached: 65536.
    
    Test Plan:
    make fast_tests
    run server mode flib test and make sure that no warning as shown above.
    
    Reviewers: qigao, mwilliams
    
    Reviewed By: qigao
    
    CC: hphp-diffs@lists, ps, mwilliams, qigao
    
    Differential Revision: 350451
  2. @scottmac

    [Perf/Fix/HPHPi] Use the method table in class eval state for lookup

    myang authored scottmac committed
    Summary:
    Each class statement AST node has a method table that contains only the
    methods in the class, but not any of the inherited methods. When a class is
    eval'ed, a ClassEvalState is created which contains among other things all
    the methods that the class should have, including all the inherited methods.
    When resolving a static class method call (i.e., finding a class method for
    a given class name), we can take advantage of the inclusive method table in
    the ClassEvalState, rather than redo the search along the class hierarchy
    for methods in parent classes.
    
    Second, I noticed that the m_marker in ClassStatement is always set. From the
    comments it meant to indicate whether the class is top-level and therefore
    should be hoisted upfront. I fixed that and there was one test failing because
    HPHPi starts to raise error as expected.
    
    Test Plan:
    make fast_tests
    flib/unit test
    (1) Error case:
    
    <?php
    $a = bar();
    if ($a) {
      class fOO extends Unknown {}
    } else {
      class Foo extends unknOwn {}
    }
    function bar() { return 123; }
    
    Before: no error reported
    After:
    HipHop Fatal error: Class 'Unknown' does not exist.
    
    (2) micro benchmark
    <?php
    class A0 {
      public static function __callStatic($name, $arguments) {
         return $name;
      }
    }
    class A1 extends A0 {}
    class A2 extends A1 {}
    class A3 extends A2 {}
    class A4 extends A3 {}
    class A5 extends A4 {}
    class A6 extends A5 {}
    class A7 extends A6 {}
    function foo() {
      for ($i = 0; $i < 1000000; $i++) {
        $r = A7::Test();
      }
      return $r;
    }
    var_dump(foo());
    
    Before
    string(4) "Test"
    
    real    0m6.238s
    user    0m6.208s
    sys     0m0.035s
    
    After
    string(4) "Test"
    
    real    0m2.783s
    user    0m2.757s
    sys     0m0.033s
    
    Reviewers: mwilliams, qigao
    
    Reviewed By: qigao
    
    CC: hphp-diffs@lists, ps, mwilliams, qigao
    
    Differential Revision: 349254
  3. @scottmac

    [Fix] Fix bug in FileCache::fileSize

    myang authored scottmac committed
    Summary: When uncompressing the file, we should use buf.clen instead of
    buf.len.
    
    Test Plan:
    make fast_tests
    make slow_tests
    
    Reviewers: qigao, mwilliams, ottoni
    
    Reviewed By: qigao
    
    CC: hphp-diffs@lists, ps, mwilliams, qigao
    
    Differential Revision: 349850
    
    Task ID: 754631
  4. @markw65 @scottmac

    [Fix] Dont pass invalid keys to ArrayData::remove

    markw65 authored scottmac committed
    Summary: Variant::remove(CVarRef) could end up passing a "null" key to ArrayData::remove if given an invalid key (array or object). In the DEBUG build, this asserted, and in the release build it removed the element with key 0.
    
    The new test case produced incorrect results in the release build, and asserted in the debug build.
    
    Task ID: #768801
    
    Blame Rev:
    
    Reviewers: kma qigao
    
    CC:
    
    Test Plan: fast_tests slow_tests (new test case added).
    
    Revert Plan:
    
    Tags:
    
    Platform Impact (PUBLIC):
    
    Differential Revision: 347370
Commits on Nov 2, 2011
  1. @scottmac

    Debugger Server API

    qigao authored scottmac committed
    Summary:
    To build GUI for hphpd, we need to access DebuggerClient using an API.
    This diff changes the existing debugger extension to do that. The
    current built-in class DebuggerClient and DebuggerProxy are just to
    support CmdUser, so they are renamed with suffix CmdUser. Then a new
    extension class is added to wrap around DebuggerClient
    for executing commands. An extension function is added to fetch
    DebuggerClient by name so that multiple requests can operate on the same
    client for debugging. This way, a GUI tool can use Ajax request to hit
    the server and operate on debugger via this API.
    
    Since this change makes DebuggerClient running in server mode as root,
    it breaks a few assumptions in DebuggerClient, so several other
    modifications are added to make sure:
    - the sweeping after request doesn't break anything,
    - multiple DebuggerClient can co-exist on the same server without
    interfering each other
    - credentials do not rely on local login and home
    - disable color and highlighting since it's not terminal printing
    
    This diff also modifies the Makefile for idl the same way as Makfile for
    system so that it doesn't give a lot of warnings.
    
    Some more work need to be done for better handling error and corner
    cases. Also need to find a way to format output. For now it just
    returns the string printed by DebuggerClient after each processCmd().
    
    Test Plan:
    make fast_tests
    tried most functionality with a debug php endpoint on sandbox
    
    Reviewers: myang, mwilliams
    
    Reviewed By: myang
    
    CC: tuomaspelkonen, ps, mwilliams, myang
    
    Differential Revision: 346215
    
    Task ID: 753544
Commits on Nov 1, 2011
  1. @scottmac

    Fix segfault in SimpleXML when using entities

    scottmac authored
    Summary:
    Fix segfault while looking at security issue, any XML
    file parsed by SimpleXML resulted in a segfault if an Entity was
    provided.
    
    Test Plan:
    fast_tests
    No segfault with:
    <?php
    
    $test = '<!DOCTYPE foo [<!ENTITY xxe SYSTEM "file:///etc/passwd">]>
    <search><user>&xxe;</user></search>';
    $x = new SimpleXMLElement($test);
    var_dump($x);
    
    Reviewers: mwilliams
    
    Reviewed By: mwilliams
    
    CC: ps, mwilliams
    
    Differential Revision: 346984
    
    Revert Plan: Ok
  2. @scottmac

    Export psp CPU and Wall time to access log

    ps authored scottmac committed
    Summary:
    Getting this timing inside php is difficult and not accurate.
    Provide this timing inside of HPHP via %Z and %z.
    
    Test Plan:
    added %Z %z in the access log and compared to timings
    calculated in php and they were slighly higher, which is expected.
    
    Reviewers: qigao
    
    Reviewed By: qigao
    
    CC: jasont, ps, mwilliams, qigao
    
    Differential Revision: 346858
    
    Task ID: 755067
  3. @markw65 @scottmac

    [Fix] Unravel gotos into try blocks correctly

    markw65 authored scottmac committed
    Summary: At each try, there can be multiple label-ids for each label (but
    exactly one label for each label-id). Swap the map to map label-ids to labels,
    rather than the other way around to avoid losing label-ids.
    
    Test Plan: fast_tests slow_tests
    
    Reviewers: myang, qigao, andrewparoski
    
    Reviewed By: andrewparoski
    
    CC: ps, mwilliams, andrewparoski
    
    Differential Revision: 346349
    
    Task ID: 765340
Commits on Oct 18, 2011
  1. @scottmac

    Fix build

    scottmac authored
  2. @markw65 @scottmac

    [Perf] Reduce temporary generation

    markw65 authored scottmac committed
    Summary:
    Fix a few issues:
     - if any child node had a side effect, and produces a temporary anyway, we
    would enforce ordering. But this isnt needed for a single child node, and is
    counter-productive for eg isset and empty.
     - if we wanted to output a fast-cast, we would force a temporary. But this is
    wrong in exist context for ArrayElementExpression and ObjectProperty expression
    (the temporary will be evaluated using rvalAt, rather than isset/empty). So in
    these cases, we would emit both an rvalAt, and an isset/empty.
     - type inference was wrong for static method calls to non-static functions,
    when there is (or might be) no object available (it failed to set
    implementedType to Variant, which it needs to because the call will be
    dispatched via an invoke helper).
    
    Test Plan: fast_tests slow_tests
    
    Reviewers: myang, qigao
    
    Reviewed By: myang
    
    CC: ps, mwilliams, myang
    
    Differential Revision: 345684
  3. @markw65 @scottmac

    [Fix] declareConstant should return true on success

    markw65 authored scottmac committed
    Summary:
    declareConstant returned false for illegal values, but the defined value for
    everything else. It should return true for success, and false otherwise.
    
    Also add warning for the redeclared case, since it should be rare.
    
    Test Plan: fast_tests slow_tests
    
    Reviewers: myang, qigao
    
    Reviewed By: myang
    
    CC: ps, mwilliams, myang
    
    Differential Revision: 345637
  4. @scottmac

    [Fix] Do not set HTTP_RAW_POST_DATA if it cause String overflow

    myang authored scottmac committed
    Summary:
    In video upload, the amount of data can overflow a String and cause exception
    to be throw. I made a change so that we don't store HTTP_RAW_POST_DATA when
    this happens.
    
    Test Plan:
    make fast_tests
    upload a large video in non-chunk mode and make sure no exception is thrown
    at the end.
    
    Reviewers: mwilliams, qigao
    
    Reviewed By: mwilliams
    
    CC: hphp-diffs@lists, ps, mwilliams
    
    Differential Revision: 345263
  5. @scottmac

    [Perf/Size] Uninline MethodCallPackage constructor

    myang authored scottmac committed
    Summary:
    There are many calls to MethodCallPackage constructor in the generated code.
    I unlined it and checked the size and performance.
    
    Test Plan:
    make fast_tests
    make slow_tests
    build www run production test.
    
    Reviewers: mwilliams, qigao
    
    Reviewed By: mwilliams
    
    CC: hphp-diffs@lists, ps, mwilliams, myang
    
    Differential Revision: 344702
  6. @markw65 @scottmac

    [Cleanup] Better support for OUTPUT_ROOT

    markw65 authored scottmac committed
    Summary: The test binary was hard coded to expect hphpi at src/hphpi/hphpi. Fix
    it to take the path via a define at compile time, rather than copying the binary
    there from OUTPUT_ROOT. A couple of other small fixes.
    
    Test Plan: fast_tests slow_tests with/without OUTDIR_BY_TYPE set
    
    Reviewers: qigao, myang
    
    Reviewed By: myang
    
    CC: ps, mwilliams, myang
    
    Differential Revision: 344296
  7. @markw65 @scottmac

    [Perf] More efficient handling of __callStatic

    markw65 authored scottmac committed
    Summary: Store HasCall and HasCallStatic flags in ObjectStaticCallbacks for
    hphp, in ClassStatement and ClassEvalState for hphpi, and in ClassInfo (as a
    last resort) to allow testing without creating an object. Also precompute
    UseGet, UseSet, UseIsset and UseUnset for hphpi, rather than calling findMethod
    for each one on each object construction.
    
    Test Plan: fast_tests slow_tests
    
    Reviewers: myang
    
    Reviewed By: myang
    
    CC: cbueno, ps, mwilliams, myang
    
    Differential Revision: 344089
    
    Task ID: 746527
  8. @markw65 @scottmac

    [Perf] Cleanup uses of create_object

    markw65 authored scottmac committed
    Summary: Various callers of create_object were passing a const char *, rather
    than a CStrRef, usually via str.data(), causing unnecessary work. Also, several
    call sites were passing an empty or null array, with the init param set to
    false. Replace with calls to create_object_only, which saves the array
    construction overhead.
    
    Test Plan: fast_tests slow_tests
    
    Reviewers: qigao, myang, je
    
    Reviewed By: myang
    
    CC: ps, mwilliams, myang
    
    Differential Revision: 343975
  9. @markw65 @scottmac

    [Perf] Dont fetch globals unless needed

    markw65 authored scottmac committed
    Summary: get_call_info declares the globals up front, but rarely needs them.
    
    Test Plan: fast_tests slow_tests
    
    Reviewers: myang, qigao
    
    Reviewed By: myang
    
    CC: ps, mwilliams, myang
    
    Differential Revision: 343267
  10. @markw65 @scottmac

    [Perf] Dont duplicate the function lookup in fb_call_user_func_safe*

    markw65 authored scottmac committed
    Summary: fb_call_user_func_safe etc were implemented via is_callable, then
    call_user_func_array. Instead, we can do a single lookup of the CallInfo.
    
    Test Plan: fast_tests slow_tests
    
    Reviewers: myang, qigao
    
    Reviewed By: myang
    
    CC: ps, mwilliams, myang
    
    Differential Revision: 343223
Something went wrong with that request. Please try again.